Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Refactor migration with ctx manager to diable sqlite fkey #51392

wants to merge 4 commits into from

Conversation

jason810496
Copy link
Member

@jason810496 jason810496 commented Jun 4, 2025 •
edited
Loading

related: #51336

What

Follow-up PR for #51336 (comment) comment, use disable_sqlite_fkeys context manager instead of manual call for disabling SQLite Foreign Key constraint in db-migrations.

@jason810496 jason810496 requested a review from ephraimbuddy as a code owner June 4, 2025 07:54
@jason810496 jason810496 requested a review from Lee-W June 4, 2025 07:55
@ephraimbuddy
Copy link
Contributor

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

@ephraimbuddy
Copy link
Contributor

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

@jason810496
Copy link
Member Author

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 ? Or where can I specify the test for sqlite? On GitHub Action, area labels or where should I set it ?

@ephraimbuddy
Copy link
Contributor

https://github.com/apache/airflow/blob/main/.github/actions/migration_tests/action.yml

It's just to remove this line:

airflow/.github/actions/migration_tests/action.yml

Line 41 in 0911676

if: env.BACKEND != 'sqlite'
and this:

airflow/.github/actions/migration_tests/action.yml

Line 63 in 0911676

if: env.BACKEND != 'sqlite'

@jason810496 jason810496 force-pushed the refactor/db-migrations/replace-manual-call-with-disable_sqlite_fkeys branch from 74121d2 to 891df2c Compare June 10, 2025 14:19
@jason810496 jason810496 marked this pull request as draft June 11, 2025 09:38
@jason810496 jason810496 force-pushed the refactor/db-migrations/replace-manual-call-with-disable_sqlite_fkeys branch from 95c6b51 to c272f1c Compare June 11, 2025 09:45
@jason810496 jason810496 force-pushed the refactor/db-migrations/replace-manual-call-with-disable_sqlite_fkeys branch from 20766e0 to df1672f Compare June 24, 2025 15:10
@jason810496 jason810496 marked this pull request as ready for review June 25, 2025 08:08
@jason810496 jason810496 marked this pull request as draft June 25, 2025 08:08
@jason810496 jason810496 force-pushed the refactor/db-migrations/replace-manual-call-with-disable_sqlite_fkeys branch from df1672f to dc46f7a Compare June 29, 2025 10:20
@jason810496 jason810496 marked this pull request as ready for review June 29, 2025 10:20
@jason810496 jason810496 marked this pull request as draft June 29, 2025 13:56
@jason810496 jason810496 force-pushed the refactor/db-migrations/replace-manual-call-with-disable_sqlite_fkeys branch from dc46f7a to 5f63916 Compare June 30, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lee-W Lee-W Lee-W approved these changes

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

Assignees
No one assigned
Labels
area:db-migrations PRs with DB migration kind:documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jason810496 @ephraimbuddy @Lee-W