Skip to content

Conversation

@uranusjr
Copy link
Member

These checks were put into place early in 2.x to add stricter constraints in the database. They are executed before the migrations when the user runs 'airflow upgradedb' to make sure there are no data integrity issues that would fail the migration halfway, which is both difficult to debug and recover from.

The checks always pass if the user is on a more recent Airflow 2 version, on which the constraints are now enforced by the database. Since we have removed all migration files prior to 2.10, the user must upgrade to at least 2.10 first (which has the checks) before upgrading to 3.0 anyway, so we can safely remove the checks going forward.

(Additional context: these checks need to be modified if we are to rename execution_date on DagRun, which I’m doing in #41818. Instead of needing to work on the change, I think we should remove those checks since they don’t ever need to be run anyway.)

@uranusjr
Copy link
Member Author

I decided to keep the mechanism around integrity checks since we’re very likely to need it again at some point in the future. Only the actual checks were removed (so the mechanism simply runs an empty loop and exits).

These checks were put into place early in 2.x to add stricter
constraints in the database. They are executed before the migrations
when the user runs 'airflow upgradedb' to make sure there are no data
integrity issues that would fail the migration halfway, which is both
difficult to debug and recover from.

The checks always pass if the user is on a more recent Airflow 2
version, on which the constraints are now enforced by the database.
Since we have removed all migration files prior to 2.10, the user must
upgrade to at least 2.10 first (which has the checks) before upgrading
to 3.0 anyway, so we can safely remove the checks going forward.
@uranusjr uranusjr force-pushed the remove-db-upgrade-checks branch from e8506f4 to 381f54a Compare August 29, 2024 07:27
Copy link
Collaborator

@sunank200 sunank200 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM.

@uranusjr uranusjr merged commit 56a3987 into apache:main Aug 29, 2024
@uranusjr uranusjr deleted the remove-db-upgrade-checks branch August 29, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants