-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Refactor migration with ctx manager to diable sqlite fkey #51392
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
Refactor migration with ctx manager to diable sqlite fkey #51392
Conversation
|
You can enable the CI migration test for sqlite here: https://github.com/apache/airflow/blob/main/.github/actions/migration_tests/action.yml. Except for the offline migration |
Please let's enable this test before merge |
Hi @ephraimbuddy, sorry that I still can't get it. |
It's just to remove this line:
|
74121d2 to
891df2c
Compare
95c6b51 to
c272f1c
Compare
20766e0 to
df1672f
Compare
df1672f to
dc46f7a
Compare
dc46f7a to
5f63916
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It take some try and error to set export _AIRFLOW__SKIP_DATABASE_EXECUTOR_COMPATIBILITY_CHECK='1' correct for the CI.
It's almost green except for the the following error related to FabDBManager when downgrade.
https://github.com/apache/airflow/actions/runs/15989887297/job/45101414867?pr=51392
| return | ||
| if not inspect(settings.engine).has_table("ab_user") and not unitest_mode: | ||
| if not inspect(session.get_bind()).has_table("ab_user") and not unitest_mode: | ||
| raise AirflowException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It raise the following traceback even we are really using FabDBManager in the CI.
So I suspect the utility checking "ab_user" table existed is broke.
[2025-07-01T04:42:57.817+0000] {db.py:586} INFO - Airflow database tables created
Database migrating done!
Performing downgrade with database sqlite:////root/airflow/sqlite/airflow.db
[2025-07-01T04:43:02.453+0000] {db.py:1055} INFO - Attempting downgrade to revision 405de8318b3a
Traceback (most recent call last):
File "/usr/local/bin/airflow", line 10, in <module>
sys.exit(main())
File "/opt/airflow/airflow-core/src/airflow/__main__.py", line 55, in main
args.func(args)
File "/opt/airflow/airflow-core/src/airflow/cli/cli_config.py", line 48, in command
return func(*args, **kwargs)
File "/opt/airflow/airflow-core/src/airflow/utils/cli.py", line 113, in wrapper
return f(*args, **kwargs)
File "/opt/airflow/airflow-core/src/airflow/utils/providers_configuration_loader.py", line 56, in wrapped_function
return func(*args, **kwargs)
File "/opt/airflow/airflow-core/src/airflow/cli/commands/db_command.py", line 204, in downgrade
run_db_downgrade_command(args, db.downgrade, _REVISION_HEADS_MAP)
File "/opt/airflow/airflow-core/src/airflow/cli/commands/db_command.py", line 179, in run_db_downgrade_command
command(to_revision=to_revision, from_revision=from_revision, show_sql_only=args.show_sql_only)
File "/opt/airflow/airflow-core/src/airflow/utils/session.py", line 101, in wrapper
return func(*args, session=session, **kwargs)
File "/opt/airflow/airflow-core/src/airflow/utils/db.py", line 1070, in downgrade
raise AirflowException(
airflow.exceptions.AirflowException: Downgrade to revision less than 3.0.0 requires that `ab_user` table is present. Please add FabDBManager to [core] external_db_managers and run fab migrations before proceeding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some of the tests, there's a setting that adds external db managers, probably the failing test is missing the setting
09f1d60 to
f483db1
Compare
f483db1 to
7fe74c3
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
related: #51336
What
Follow-up PR for #51336 (comment) comment, use
disable_sqlite_fkeyscontext manager instead of manual call for disabling SQLite Foreign Key constraint in db-migrations.