Fix external DB manager upgrades with existing tables#66674
Conversation
3ec0bcd to
901b519
Compare
jscheffl
left a comment
There was a problem hiding this comment.
In general GOOD! Thanks for the fix - actually ran into the same myself but had no time in discovering the root. But now seeing the PR this is opening my eyes!
Can you fix the nit?
Any maybe would be good to split the PRs such that we can make the airflow-core diff as backport into Airflow 3.2.2
|
Hi maintainer, this PR was merged without a milestone set.
|
|
@anmolxlight thanks for the patch! As I requested to remove Providers I am not able to back-port such that it can get into 3.2.2. Are you planning to revamp another PR with the earlier proposed changes in providers? Would be great! (Alternatively I could but would leave the credit rather to you :-D) |
Backport successfully created: v3-2-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
|
Yes, I will open a separate provider-only PR restoring the edge3/FAB compatibility changes that were removed here. I will keep it scoped to providers and reference this PR for the core behavior. |
|
Opened the provider-only follow-up here: #66883. It restores the Edge3/FAB compatibility path only, with regression coverage for the existing-provider-tables/no-Alembic-version case. |
…6674) (#66882) * Fix external DB manager upgrades with existing tables * Fix provider db manager upgrade compatibility * Add provider DB manager compatibility comments * Clarify provider DB manager cleanup version * Fix Kafka trigger sync executor contention * Stabilize Kafka message queue trigger test * fix: narrow db manager fix to core --------- (cherry picked from commit 671dd4c) Co-authored-by: Anmol Mishra <anmolxlight@gmail.com> Co-authored-by: Anmol Mishra <anmolx.work@gmail.com>
…6674) (#66882) * Fix external DB manager upgrades with existing tables * Fix provider db manager upgrade compatibility * Add provider DB manager compatibility comments * Clarify provider DB manager cleanup version * Fix Kafka trigger sync executor contention * Stabilize Kafka message queue trigger test * fix: narrow db manager fix to core --------- (cherry picked from commit 671dd4c) Co-authored-by: Anmol Mishra <anmolxlight@gmail.com> Co-authored-by: Anmol Mishra <anmolx.work@gmail.com>
…6674) (#66882) * Fix external DB manager upgrades with existing tables * Fix provider db manager upgrade compatibility * Add provider DB manager compatibility comments * Clarify provider DB manager cleanup version * Fix Kafka trigger sync executor contention * Stabilize Kafka message queue trigger test * fix: narrow db manager fix to core --------- (cherry picked from commit 671dd4c) Co-authored-by: Anmol Mishra <anmolxlight@gmail.com> Co-authored-by: Anmol Mishra <anmolx.work@gmail.com>
…6674) (#66882) * Fix external DB manager upgrades with existing tables * Fix provider db manager upgrade compatibility * Add provider DB manager compatibility comments * Clarify provider DB manager cleanup version * Fix Kafka trigger sync executor contention * Stabilize Kafka message queue trigger test * fix: narrow db manager fix to core --------- (cherry picked from commit 671dd4c) Co-authored-by: Anmol Mishra <anmolxlight@gmail.com> Co-authored-by: Anmol Mishra <anmolx.work@gmail.com>
What
Fixes external DB manager upgrades when manager-owned tables already exist but the manager-specific Alembic version table does not.
Previously,
BaseDBManager.upgradedb()treated that state as an empty database, calledcreate_db_from_orm(), and stamped the manager to head. Becausemetadata.create_all()is a no-op for existing tables, migration bodies were skipped while Alembic believed the schema was current.Why
This breaks upgrades such as Airflow 3.1 + edge3 1.x to Airflow 3.2 + edge3 3.4, where
edge_job,edge_worker, andedge_logsalready exist from the pre-EdgeDBManagerera, butalembic_version_edge3does not. The old path stamped edge3 to head without applying migrations that add columns likeedge_worker.concurrencyandedge_job.team_name.How
BaseDBManager.upgradedb().FABDBManager.upgradedb(), which has its own upgrade implementation.BaseDBManagertests plus edge3 and FAB coverage.Tests
uv run ruff check airflow-core/src/airflow/utils/db_manager.py airflow-core/tests/unit/utils/test_db_manager.py providers/edge3/tests/unit/edge3/models/test_db.py providers/fab/src/airflow/providers/fab/auth_manager/models/db.py providers/fab/tests/unit/fab/auth_manager/models/test_db.pyuv run --project airflow-core pytest airflow-core/tests/unit/utils/test_db_manager.py -quv run --project providers/edge3 pytest providers/edge3/tests/unit/edge3/models/test_db.py -quv run --project providers/fab pytest providers/fab/tests/unit/fab/auth_manager/models/test_db.py -qFixes: #66524