-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Allow downgrading to 2.11 from 3.x #54231
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
Conversation
|
Hmm the test failures here are interesting -- it's cause downgrade is being called twice, once for the base/airflow-core and once for the DB manager, which is now finding a revision. Which might mean that Bah Humbug. |
|
Those "external DB managers" is a real pain - I am happy we opposed to take it further for the AWS provider. I'd say the only good way of solving those is to separate it to two commands that have nothing to do with each other and are individually run by the users. Not sure if this is doable now, |
|
@o-nikolas any insight in to this? |
|
My current thought after stepping away from this for a few hours is to change the syntax of i.e. turn to-revision from nargs=1 to nargs=+, and then split on Thoughts? |
|
Oh! This is just a bad test! The |
88576c5 to
18a5e60
Compare
18a5e60 to
45b67c3
Compare
There were two things blocking this: 1) The revision heads map didn't have any 2.11.x versions in it, so the previous implementation of `_get_version_revision` was only looking within the same <major.minor> pathc version. We change it to rely on the fact that our pre-commit checks ensure this map is ordered, and iterate over the dictionary reversed, and when we find the first thing less than the target revision we use that (direct equal is handled already above) 2) The `ab_*` tables not existing were blocking the migration. Part of this is now fixable manually with apache#54227, but I have decided that since FAB was required and the only option in 2.x, so I have decided to just create the tables if they are missing In order to try and cope with possible future changes I create the tables at the latest version and then downgrade to the oldest known revision. This is all handled in a `reset_to_2_x()` method on the FABDBManager, with a fallback to just blindly create the tables from the ORM for versions of the provider that don't yet have that function.
45b67c3 to
44a79b8
Compare
This never made sense, and wasn't actually called as part of the `airflow db downgrade` CLI calls. The reason it doesn't make sense is that the version you pass is either the Airflow version (but external DB managers are installed and versioned separately) or the migration revision ID for the Airflow Core meta db. For FAB specifically there is the `airflow fab-db` CLI command to manage things, so "checking RunDBManager doesn't run Fab migrations" doesn't make sense as a test now (as the code that _could_ do it is removed), so I've removed the test too.
44a79b8 to
96f03b9
Compare
Backport failed to create: v3-0-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker b9cdc3d v3-0-testThis should apply the commit to the v3-0-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
|
This broke main, I'll revert it for now. |
* Allow downgrading to 2.11 from 3.x There were two things blocking this: 1) The revision heads map didn't have any 2.11.x versions in it, so the previous implementation of `_get_version_revision` was only looking within the same <major.minor> pathc version. We change it to rely on the fact that our pre-commit checks ensure this map is ordered, and iterate over the dictionary reversed, and when we find the first thing less than the target revision we use that (direct equal is handled already above) 2) The `ab_*` tables not existing were blocking the migration. Part of this is now fixable manually with apache#54227, but I have decided that since FAB was required and the only option in 2.x, so I have decided to just create the tables if they are missing In order to try and cope with possible future changes I create the tables at the latest version and then downgrade to the oldest known revision. This is all handled in a `reset_to_2_x()` method on the FABDBManager, with a fallback to just blindly create the tables from the ORM for versions of the provider that don't yet have that function. * Remove `downgrade` from the RunDBManager interface This never made sense, and wasn't actually called as part of the `airflow db downgrade` CLI calls. The reason it doesn't make sense is that the version you pass is either the Airflow version (but external DB managers are installed and versioned separately) or the migration revision ID for the Airflow Core meta db. For FAB specifically there is the `airflow fab-db` CLI command to manage things, so "checking RunDBManager doesn't run Fab migrations" doesn't make sense as a test now (as the code that _could_ do it is removed), so I've removed the test too.
) This reverts commit b9cdc3d.
There were two things blocking this:
The revision heads map didn't have any 2.11.x versions in it, so the
previous implementation of
_get_version_revisionwas only looking withinthe same
<major>.<minor>patch version.We change it to rely on the fact that our pre-commit checks ensure this map
is ordered, and iterate over the dictionary reversed, and when we find the
first thing less than the target revision we use that (direct equal is
handled already above)
The
ab_*tables not existing were blocking the migration. Part of this isnow fixable manually with Create FAB's user/role tables on migration, not only on initdb #54227, but I have decided that since FAB was
required and the only option in 2.x, so I have decided to just create the
tables if they are missing
In order to try and cope with possible future changes I create the tables
at the latest version and then downgrade to the oldest known revision.
This is all handled in a
reset_to_2_x()method on the FABDBManager, witha fallback to just blindly create the tables from the ORM for versions of
the provider that don't yet have that function.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.