Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use DB where possible for quicker airflow dag subcommands #21793

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

ashb
Copy link
Member

@ashb ashb commented Feb 24, 2022

Some of the subcommands here don't actually need a full dag, so there is
no point paying the (possibly long) time to parse a dagfile if we could
go directly to the DB instead.

Closes #21450


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

Some of the subcommands here don't actually need a full dag, so there is
no point paying the (possibly long) time to parse a dagfile if we could
go directly to the DB instead.
@ashb ashb requested a review from kaxil February 24, 2022 14:18
@MatrixManAtYrService
Copy link
Contributor

just curious: does this apply to airflow dags show? I haven’t set up an experiment to be sure, but my feeling was that that one was working pretty hard despite the serialized dag being available.

@ashb
Copy link
Member Author

ashb commented Feb 24, 2022

No, some I chose to leave respecting the dag-as-parsed, and dag show felt like one of those

@kaxil kaxil merged commit dade6e0 into apache:main Feb 24, 2022
@kaxil kaxil deleted the dag-command-no-parse-dags branch February 24, 2022 21:45
@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Feb 24, 2022
@potiuk
Copy link
Member

potiuk commented Feb 24, 2022

Nice one 👍

rustikk pushed a commit to rustikk/airflow that referenced this pull request Feb 25, 2022
…#21793)

Some of the subcommands here don't actually need a full dag, so there is
no point paying the (possibly long) time to parse a dagfile if we could
go directly to the DB instead.

Closes apache#21450
@jedcunningham jedcunningham added the type:improvement Changelog: Improvements label Feb 28, 2022
@atishkathpal
Copy link

@ashb this is very useful. Worth considering adding this to all v2.0+ releases rather than just the upcoming release ?

@ashb
Copy link
Member Author

ashb commented Mar 26, 2022

@ashb this is very useful. Worth considering adding this to all v2.0+ releases rather than just the upcoming release ?

On the open source project we only maintain one version (currently 2.2.x), and only back-port bug fixes to it

cc @ephraimbuddy who is preparing the release for 2.2.5 - it could be viewed as a (performance) bug fix

@potiuk
Copy link
Member

potiuk commented Mar 26, 2022

@ashb this is very useful. Worth considering adding this to all v2.0+ releases rather than just the upcoming release ?

BTW. I recommend you to upgrade to latest versions quickly :) I know it is not an easy one but the more often you do it, the less scary it is and the more up-to-date/secure system you have.

@ephraimbuddy ephraimbuddy added this to the Airflow 2.2.5 milestone Mar 26, 2022
@ephraimbuddy ephraimbuddy added type:bug-fix Changelog: Bug Fixes and removed type:improvement Changelog: Improvements labels Mar 26, 2022
@ephraimbuddy ephraimbuddy added type:improvement Changelog: Improvements and removed type:bug-fix Changelog: Bug Fixes labels Mar 26, 2022
@atishbits
Copy link

@ashb this is very useful. Worth considering adding this to all v2.0+ releases rather than just the upcoming release ?

BTW. I recommend you to upgrade to latest versions quickly :) I know it is not an easy one but the more often you do it, the less scary it is and the more up-to-date/secure system you have.

Agree :). I'm using AWS MWAA so I don't have the privilege to keep my Airflow cluster updated with latest release until it shows up on the AWS console as a supported Airflow version.

leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this pull request Jun 4, 2022
Use DB where possible for quicker airflow dag subcommands
cherry-picked from apache/airflow#21793

RELNOTES=BUGFIX

Change-Id: I07c0741db5450a6e59ae0c73af83f76c21c664ac
GitOrigin-RevId: 24db62d8b3ed5d145b886e2e9520692d34b1c1e7
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this pull request Jun 6, 2022
Use DB where possible for quicker airflow dag subcommands
cherry-picked from apache/airflow#21793

RELNOTES=BUGFIX

Change-Id: I34fbdcfdb78ef7e1d9d8019f17c81d2f831da41f
GitOrigin-RevId: 9112512f6c3a381e7cde48c993195a1b4d369950
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this pull request Jun 6, 2022
Use DB where possible for quicker airflow dag subcommands
cherry-picked from apache/airflow#21793

RELNOTES=BUGFIX

Change-Id: I8abf768f45d4c9c6143858216d25f81bc66612dd
GitOrigin-RevId: 3da6ce1e8ec8b8cfd3d8cd76f6955e053c2cf1d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI okay to merge It's ok to merge this PR as it does not require more tests type:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

airflow dags status fails if parse time is near dagbag_import_timeout
8 participants