fix(migrations): merge divergent Alembic heads into a single chain - #42878
Conversation
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #b7ec72
Actionable Suggestions - 1
-
superset/migrations/versions/2026-08-06_00-01_d7cecc48bd55_merge_databend_sslmode_with_pivot_.py - 1
- Invalid module name with special characters · Line 1-1
Review Details
-
Files reviewed - 1 · Commit Range:
367600b..367600b- superset/migrations/versions/2026-08-06_00-01_d7cecc48bd55_merge_databend_sslmode_with_pivot_.py
-
Files skipped - 0
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| @@ -0,0 +1,35 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
The filename contains invalid characters (hyphens, colons) which violate Python module naming conventions. Rename the file to use underscores instead.
Code Review Run #b7ec72
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
The filename is correct for migrations in this repo
There was a problem hiding this comment.
The suggestion to rename the migration file is not necessary in this context. Migration files in this repository follow a specific naming convention that includes timestamps and identifiers, which are standard for database migration tools. You can safely ignore this suggestion.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #42878 +/- ##
==========================================
- Coverage 73.55% 66.36% -7.19%
==========================================
Files 1928 2856 +928
Lines 83529 161010 +77481
Branches 27227 37033 +9806
==========================================
+ Hits 61439 106859 +45420
- Misses 22090 52138 +30048
- Partials 0 2013 +2013
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mikebridge
left a comment
mikebridge
left a comment
There was a problem hiding this comment.
Verified against master's current migration graph — this is the correct and complete fix:
- Master's two heads are exactly
c4a1b8e2d739(Databend, chained off1a27941d5352at 13:28 UTC yesterday) and4f145192b583(#41714's merge revision, also descending from1a27941d5352, merged at 20:22 on CI that predated Databend by ~13h — the classic second-to-merge stale-lineage race).down_revision = ("c4a1b8e2d739", "4f145192b583")merges precisely that pair; with this file the graph has a single head by construction. - The open bito thread is a false positive twice over: the filename contains no colons, and the hyphenated date prefix is this repo's documented migration naming convention (
YYYY-MM-DD_HH-MM_hash_description.py) used by every sibling file, including the two this revision merges. Alembic loads these by path, not import, so module-identifier rules don't apply.
One downstream consumer to be aware of: #42863 (approved) carries a migration chained off c4a1b8e2d739 and will re-point onto d7cecc48bd55 as soon as this lands — so a quick merge here directly unblocks an already-approved PR plus the flag-flip branch (#42801).
Disclosure: verification performed with AI assistance (Claude), on behalf of and reviewed by @mikebridge.
…55 head-merge from apache#42878)
…merge b8d2f4a6c901 chained off c4a1b8e2d739, which was one of the two divergent heads that apache#42878 merged into d7cecc48bd55. Chaining off the pre-merge revision would have re-forked the graph on merge; the index migration now sits on the repaired single head. Content unchanged.
…merge b8d2f4a6c901 chained off c4a1b8e2d739, which was one of the two divergent heads that apache#42878 merged into d7cecc48bd55. Chaining off the pre-merge revision would have re-forked the graph on merge; the index migration now sits on the repaired single head. Content unchanged.
|
Attempting to prevent this in the future: #42890 |
…ier merge d7cecc48bd55 and befa892fa3ad were both independently-created merge migrations reconciling the exact same two divergent parents (4f145192b583, c4a1b8e2d739) -- one from this branch's own earlier "merge divergent heads" commit, one from master's own concurrent fix (#42878). Master converged on d7cecc48bd55 as canonical; merging master into this branch left both in the same chain, producing two alembic heads and breaking every job that runs `superset db upgrade` (docker-build, test-postgres/mysql/sqlite, E2E, Presto/Hive, CLI tests). Drops the redundant befa892fa3ad, which nothing else references. Verified locally: `flask db heads` reports exactly one head (d7cecc48bd55, matching master), and a real `superset db upgrade` against a fresh sqlite db runs the full chain cleanly end to end.
…ier merge d7cecc48bd55 and befa892fa3ad were both independently-created merge migrations reconciling the exact same two divergent parents (4f145192b583, c4a1b8e2d739) -- one from this branch's own earlier "merge divergent heads" commit, one from master's own concurrent fix (#42878). Master converged on d7cecc48bd55 as canonical; merging master into this branch left both in the same chain, producing two alembic heads and breaking every job that runs `superset db upgrade` (docker-build, test-postgres/mysql/sqlite, E2E, Presto/Hive, CLI tests). Drops the redundant befa892fa3ad, which nothing else references. Verified locally: `flask db heads` reports exactly one head (d7cecc48bd55, matching master), and a real `superset db upgrade` against a fresh sqlite db runs the full chain cleanly end to end.
5 participants
SUMMARY
Master currently has two Alembic migration heads (
4f145192b583andc4a1b8e2d739), which breaksflask db upgrade/superset db upgradeon any fresh install or existing deployment picking up both changes.The two heads were introduced by unrelated PRs that both branched off the same parent revision (
1a27941d5352) and merged to master independently, so neither author's migration graph reflected the other's new head at merge time:4f145192b583— added restoring pivot-table percent-display (merges with an earlier report-retry/compare-chart merge head)c4a1b8e2d739— migrates Databend connections to an explicitsslmodeThis PR adds a standard no-op Alembic merge revision joining both heads back into a single linear chain, following the same pattern as prior merge migrations in this repo (empty
upgrade/downgrade).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — migration-only change.
TESTING INSTRUCTIONS
flask db headson master (before this PR) reports two heads:4f145192b583andc4a1b8e2d739.flask db headsreports a single head:d7cecc48bd55.flask db upgraderuns cleanly through the merged chain (the merge revision is a no-op, so both prior migrations'upgrade()/downgrade()are unaffected).ADDITIONAL INFORMATION