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

Slightly faster downgrade tests #26939

Merged
merged 2 commits into from
Oct 14, 2022

Conversation

ashb
Copy link
Member

@ashb ashb commented Oct 7, 2022

There are two changes here:

  1. Don't reserialize the dags. It's not part of the test so just adds extra time

  2. Use cached_app in the migrations, rather than creating three copies of the app.

    To ensure this has no side-effect outside of migrations we also purge the cached app at the end of running migrations, but only if it was already imported.
    ll conflict)

@ashb ashb force-pushed the slightly-faster-downgrade-tests branch from a7e2e9b to b12b9dd Compare October 7, 2022 16:39
@ashb ashb marked this pull request as ready for review October 7, 2022 16:42
@ashb ashb requested review from potiuk and kaxil as code owners October 7, 2022 16:42
@ashb
Copy link
Member Author

ashb commented Oct 7, 2022

Locally this saved about 15s for me -- not huge, but it's not a very complex change either so I think worth it

@ashb ashb force-pushed the slightly-faster-downgrade-tests branch from b12b9dd to fe7f2bf Compare October 13, 2022 16:49
There are two changes here:

1. Don't reserialize the dags. It's not part of the test so just adds
   extra time

2. Use `cached_app` in the migrations, rather than creating three copies
   of the app.

   To _ensure_ this has no side-effect outside of migrations we also
   purge the cached app at the end of running migrations, but only if it
   was already imported.
@ashb ashb force-pushed the slightly-faster-downgrade-tests branch from fe7f2bf to dbca09f Compare October 13, 2022 17:01
@jedcunningham jedcunningham merged commit 1449ebb into apache:main Oct 14, 2022
@jedcunningham jedcunningham deleted the slightly-faster-downgrade-tests branch October 14, 2022 14:08
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.2 milestone Oct 18, 2022
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Oct 18, 2022
ephraimbuddy pushed a commit that referenced this pull request Nov 10, 2022
There are two changes here:

1. Don't reserialize the dags. It's not part of the test so just adds
   extra time

2. Use `cached_app` in the migrations, rather than creating three copies
   of the app.

   To _ensure_ this has no side-effect outside of migrations we also
   purge the cached app at the end of running migrations, but only if it
   was already imported.

(cherry picked from commit 1449ebb)
ephraimbuddy pushed a commit that referenced this pull request Nov 10, 2022
There are two changes here:

1. Don't reserialize the dags. It's not part of the test so just adds
   extra time

2. Use `cached_app` in the migrations, rather than creating three copies
   of the app.

   To _ensure_ this has no side-effect outside of migrations we also
   purge the cached app at the end of running migrations, but only if it
   was already imported.

(cherry picked from commit 1449ebb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI area:dev-tools kind:documentation type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants