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

Call get_current_version less often in bundle refresh loop #45999

Conversation

jedcunningham
Copy link
Member

In the bundle refresh loop, we can call get_current_version a lot less often, as 1) we can skip it for bundles that do not support versioning and 2) for those that do, we already know the version from the last time we refreshed!

Since this is a local call, this isn't a huge gain. But every little bit helps.

In the bundle refresh loop, we can call `get_current_version` a lot less
often, as 1) we can skip it for bundles that do not support versioning
and 2) for those that do, we already know the version from the last time
we refreshed!

Since this is a local call, this isn't a huge gain. But every little bit
helps.
@jedcunningham jedcunningham merged commit c600a95 into apache:main Jan 24, 2025
46 checks passed
@jedcunningham jedcunningham deleted the slight_optimization_in_bundle_refresh_loop branch January 24, 2025 21:10
potiuk added a commit to potiuk/airflow that referenced this pull request Jan 25, 2025
@potiuk
Copy link
Member

potiuk commented Jan 25, 2025

Hey @jedcunningham -> reverting it temporarily in #46037 as it seem to break main. You should redo it with "full tests needed" to double check what's going on . It seems theat bundle_model might get None in the test and it could be a side-effect of other test that was not run in the original PR or maybe some other change merged since it succeeded caused it to fail.

potiuk added a commit that referenced this pull request Jan 25, 2025
@jedcunningham jedcunningham restored the slight_optimization_in_bundle_refresh_loop branch January 27, 2025 01:40
@jedcunningham
Copy link
Member Author

Thanks. I'm pretty close to just adding "full tests" on every one of my PRs...

@potiuk
Copy link
Member

potiuk commented Jan 27, 2025

Thanks. I'm pretty close to just adding "full tests" on every one of my PRs...

We are moving fast and breaking things. Actually - until we stabilise thing, maybe it's worth to trigger "full tests" for all code that touches airlfow and "task_sdk"?

@utkarsharma2 utkarsharma2 added the type:new-feature Changelog: New Features label Jan 27, 2025
@utkarsharma2 utkarsharma2 added this to the Airflow 3.0.0 milestone Jan 27, 2025
@jedcunningham
Copy link
Member Author

There are definitely tradeoffs haha. I've probably just been more unlucky than average due to the changes I've been making though.

Generally speaking, I'm not held back by test duration though, so a bit more safety for main is a good trade I'd say.

@jedcunningham jedcunningham added AIP-66: DAG Bundle/Manifest and removed type:new-feature Changelog: New Features labels Jan 27, 2025
gpathak128 pushed a commit to gpathak128/airflow that referenced this pull request Jan 29, 2025
…45999)

In the bundle refresh loop, we can call `get_current_version` a lot less
often, as 1) we can skip it for bundles that do not support versioning
and 2) for those that do, we already know the version from the last time
we refreshed!

Since this is a local call, this isn't a huge gain. But every little bit
helps.
gpathak128 pushed a commit to gpathak128/airflow that referenced this pull request Jan 29, 2025
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
…45999)

In the bundle refresh loop, we can call `get_current_version` a lot less
often, as 1) we can skip it for bundles that do not support versioning
and 2) for those that do, we already know the version from the last time
we refreshed!

Since this is a local call, this isn't a huge gain. But every little bit
helps.
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
…45999)

In the bundle refresh loop, we can call `get_current_version` a lot less
often, as 1) we can skip it for bundles that do not support versioning
and 2) for those that do, we already know the version from the last time
we refreshed!

Since this is a local call, this isn't a huge gain. But every little bit
helps.
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
…45999)

In the bundle refresh loop, we can call `get_current_version` a lot less
often, as 1) we can skip it for bundles that do not support versioning
and 2) for those that do, we already know the version from the last time
we refreshed!

Since this is a local call, this isn't a huge gain. But every little bit
helps.
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants