Skip to content

Conversation

@ashb
Copy link
Member

@ashb ashb commented Aug 11, 2025

(This is a "re-run" of #54231 which got reverted due to failing on Python 3.13)

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> 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)

  2. The ab_* tables not existing were blocking the migration. Part of this is
    now 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, with
    a fallback to just blindly create the tables from the ORM for versions of
    the provider that don't yet have that function.

ashb added 2 commits August 11, 2025 21:46
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.
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.
@ashb ashb requested a review from vincbeck as a code owner August 11, 2025 21:11
@ashb ashb added the full tests needed We need to run full set of tests for this PR to merge label Aug 11, 2025
@ashb ashb merged commit 1d04f09 into apache:main Aug 12, 2025
111 checks passed
@ashb ashb deleted the downgrade-to-2.11 branch August 12, 2025 07:25
ashb added a commit to astronomer/airflow that referenced this pull request Aug 12, 2025
* 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.

(cherry picked from commit 1d04f09)
ashb added a commit to astronomer/airflow that referenced this pull request Aug 12, 2025
* 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.

(cherry picked from commit 1d04f09)
ashb added a commit to astronomer/airflow that referenced this pull request Aug 12, 2025
* 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.

(cherry picked from commit 1d04f09)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants