Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
github-actions[bot] commented on PR #51392: URL: https://github.com/apache/airflow/pull/51392#issuecomment-3235321783 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
ephraimbuddy commented on code in PR #51392:
URL: https://github.com/apache/airflow/pull/51392#discussion_r2204366777
##
airflow-core/src/airflow/utils/db.py:
##
@@ -1066,7 +1066,7 @@ def downgrade(*, to_revision, from_revision=None,
show_sql_only=False, session:
except ImportError:
log.warning("Import error occurred while importing
FABDBManager. Skipping the check.")
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(
Review Comment:
In some of the tests, there's a setting that adds external db managers,
probably the failing test is missing the setting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
jason810496 commented on code in PR #51392:
URL: https://github.com/apache/airflow/pull/51392#discussion_r2176602868
##
airflow-core/src/airflow/utils/db.py:
##
@@ -1066,7 +1066,7 @@ def downgrade(*, to_revision, from_revision=None,
show_sql_only=False, session:
except ImportError:
log.warning("Import error occurred while importing
FABDBManager. Skipping the check.")
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(
Review Comment:
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+] {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+] {db.py:1055} INFO - Attempting downgrade to
revision 405de8318b3a
Traceback (most recent call last):
File "/usr/local/bin/airflow", line 10, in
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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
ephraimbuddy commented on PR #51392: URL: https://github.com/apache/airflow/pull/51392#issuecomment-2958121830 > https://github.com/apache/airflow/blob/main/.github/actions/migration_tests/action.yml It's just to remove this line: https://github.com/apache/airflow/blob/0911676c287f11f37470b20d0618b1f820b3f4f6/.github/actions/migration_tests/action.yml#L41 and this: https://github.com/apache/airflow/blob/0911676c287f11f37470b20d0618b1f820b3f4f6/.github/actions/migration_tests/action.yml#L63 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
jason810496 commented on PR #51392: URL: https://github.com/apache/airflow/pull/51392#issuecomment-2944492905 > > 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. Do you mean I have to change the [action.yml](https://github.com/apache/airflow/blob/main/.github/actions/migration_tests/action.yml) ? Or where can I specify the test for sqlite? On GitHub Action, area labels or where should I set it ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
ephraimbuddy commented on PR #51392: URL: https://github.com/apache/airflow/pull/51392#issuecomment-2944452281 > 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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Refactor migration with ctx manager to diable sqlite fkey [airflow]
ephraimbuddy commented on PR #51392: URL: https://github.com/apache/airflow/pull/51392#issuecomment-2939169711 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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
