Skip to content

fix(migrations): merge purge audit and username index heads - #44288

Merged
aminghadersohi merged 1 commit into
apache:masterfrom
aminghadersohi:aminghadersohi/ch121019/fix-conflicting-alembic-heads
Sep 15, 2026
Merged

aminghadersohi merged 1 commit into
apache:masterfrom
aminghadersohi:aminghadersohi/ch121019/fix-conflicting-alembic-heads

Conversation

@aminghadersohi

@aminghadersohi aminghadersohi commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

SUMMARY

Restore a single Alembic head with one no-op merge revision. No existing migration, driver requirement, schema, or application data is changed by the new revision.

Reproduced on exact master ae240a8e6f0332acb709943d0884eae73927c9de: both ScriptDirectory.get_current_head() and flask_migrate.upgrade() fail because the graph has two heads:

Both paths descend from 7e2c9a4f1b83. New revision e2f3a1b9c640 joins them without rewriting ancestry that deployments may already have applied. This follows the documented superset db merge alternative and existing no-op merge migrations.

This addresses the shared migration-startup blocker observed on #44285, #44286, and #44287, without changing those driver PRs. The general Python-Unit jobs on all three driver PRs passed; do not conflate it with the failing migration-dependent Presto/Hive, integration, and startup jobs.

Landing coordination: #44283 already joins the same two parents inside a ClickHouse data migration. Coordination requested to keep this baseline repair independent of that data rewrite. If this PR lands first, #44283 must rebase/reparent its migration onto e2f3a1b9c640; landing both unchanged would recreate two heads. If #44283 lands first, this PR must be reassessed/dropped. No agreement or maintainer approval is implied.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable. Before: two heads and ambiguous upgrade head. After: one head, e2f3a1b9c640.

TESTING INSTRUCTIONS

  • Real Alembic graph: exact master has two heads; this change has only e2f3a1b9c640. Hosted enforce-single-migration-head passed.
  • Real SQLite migrations: upgrades from each old head, both heads, common ancestor, and an empty DB passed. Merge-only downgrade preserved schema/data and restored both parent version rows; common-ancestor downgrade/re-upgrade also passed.
  • pytest tests/unit_tests/migrations -q: 147 passed before and after.
  • Full tests/common tests/unit_tests baseline at exact master: 15,360 passed, 6 failed, 6 skipped, 2 xfailed. Failures concern subject SQL quoting, theme commit count, MCP health infrastructure, missing optional profiler, and dashboard validation—not Alembic. Post-change full suite: 15,361 passed, 5 failed, 6 skipped, 2 xfailed. All five remaining failures are also present in the baseline; the subject SQL-quoting failure did not recur, which is not attributed to this no-op migration. No new local failures and no claim of a clean local full suite.
  • Changed-file pre-commit and explicit staged mypy passed using the normal isolated hook environment. An initial run with test-only dependency overlays on PYTHONPATH caused unrelated mypy import errors; removing that overlay restored the standard hook environment. No hook or code exclusions changed.
  • Fresh independent review of exact commit d6f00dcffec9beb74bc9480b40f38515c5527a5a: no correctness findings; independently repeated real graph and SQLite transition validation. PostgreSQL/MySQL execution is left to CI/maintainer validation.

Manual verification:

  1. Run superset db heads: expect only e2f3a1b9c640.
  2. Run superset db upgrade from either old head, both old heads, their common ancestor, or an empty metadata database.
  3. From the new merge head, run superset db downgrade c7f53d184ea2: Alembic removes only the merge revision and restores both parent version rows, preserving schema/data. Use the explicit revision, not ambiguous relative -1.
  4. Run superset db upgrade again.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

The merge itself only updates Alembic version bookkeeping: negligible runtime, no application table changes or additional downtime. Previously unapplied parent migrations retain their own runtime/locking characteristics. SIP-59 maintainer approvals and review period still apply; independent automated review is not a substitute.

@bito-code-review

bito-code-review Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Agent Run #424c60

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: d6f00dc..d6f00dc
    • superset/migrations/versions/2026-09-15_06-50_e2f3a1b9c640_merge_purge_audit_and_username_index_heads.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 an incremental AI Review.

  • /review full - 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 Bito Logo

@codecov

codecov Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.24%. Comparing base (b6de64f) to head (d6f00dc).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #44288      +/-   ##
==========================================
+ Coverage   75.91%   80.24%   +4.32%     
==========================================
  Files        2926     2926              
  Lines      173023   173023              
  Branches    40139    40139              
==========================================
+ Hits       131351   138835    +7484     
+ Misses      38993    31596    -7397     
+ Partials     2679     2592      -87     
Flag Coverage Δ
hive 37.35% <ø> (?)
mysql 56.92% <ø> (?)
postgres 56.94% <ø> (?)
presto 39.45% <ø> (?)
python 84.65% <ø> (+8.51%) ⬆️
sqlite 56.64% <ø> (?)
unit 76.14% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Baseline landing coordination: #44317 proposes the same two-parent no-op merge and has a maintainer approval. #44284 is also blocked by this graph; I am monitoring for the selected repair to land, then will rebase/rerun #44284. Please coordinate #44288, #44317, and #44283 so only one parallel merge revision lands (or reparent later revisions). No merge or approval is being performed by this session.

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

CI ownership follow-up from #44287: I inspected all 15 failed jobs on its exact head da5e0f6. Twelve execution jobs fail at the multiple-head migration startup; three required aggregate jobs propagate those failures. This repair PR's live applicable checks are successful and it still requires human review. Please coordinate maintainer review/landing with #44283's migration overlap. I will not merge either PR. Once this repair lands, I will rebase #44287 onto repaired master, validate and safely push, then monitor every applicable check to terminal success.

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Live CI and review audit — 2026-09-15 18:11 UTC

Exact head: d6f00dcffec9beb74bc9480b40f38515c5527a5a.

68 SUCCESS, 13 SKIPPED, 3 NEUTRAL; no failed, cancelled, action-required, or pending check conclusions. All 13 required checks are terminal SUCCESS.

Inspected all review threads (zero, with pagination exhausted), reviews (zero), and both issue comments. Bito reports zero actionable suggestions. No review feedback needs resolution.

Checked skip conditions: six frontend child jobs are excluded for this migration-only diff; Dependabot approval/requirements sync, first-time welcome, two hold-label jobs, Showtime, and master-push-only docker-compose are inapplicable. None is required. Three Netlify informational checks report NEUTRAL/“Deploy canceled”; the deploy status is SUCCESS, and docs/netlify.toml intentionally skips builds without docs changes. These are not being relabeled as successful checks.

Fetched master at a184395eca: nine additional commits affect only unrelated frontend/dependency files, no migrations. GitHub reports MERGEABLE; no rebase/conflict resolution or code push is necessary. Changed-file pre-commit and explicit mypy passed again locally; worktree is clean and remains pinned to zone-sqlalchemy-2.

Remaining human gates: maintainer approval (REVIEW_REQUIRED, including the migration approval process) and landing-order coordination with #44283. That PR remains open at d8899a6e0743ec78861175990b7406ef891fe95f; the existing coordination request remains applicable: reparent it onto e2f3a1b9c640 if #44288 lands first, or reassess/drop #44288 if #44283 lands first. Do not land both unchanged. No merge or draft conversion performed.

Full check rollup (84 entries, including repeated workflow executions)
Workflow Check Conclusion Required
Auto-approve Dependabot patch bumps Approve patch-level bump SKIPPED No
Hold Label CI Gate Cancel CI runs when hold label applied SKIPPED No
Sync requirements for Python dependency PRs sync-python-dep-requirements SKIPPED No
Welcome New Contributor welcome SKIPPED No
🎪 Superset Showtime 🎪 Sync PR to desired state SKIPPED No
Build & publish docker images changes SUCCESS No
Check DB migration conflict Check DB migration conflict SUCCESS No
Check OpenAPI spec drift check-openapi-spec-drift SUCCESS No
Check python dependencies check-python-deps SUCCESS No
CodeQL changes SUCCESS No
Dependency Review dependency-review SUCCESS Yes
E2E changes SUCCESS No
Enforce single Alembic migration head enforce-single-migration-head SUCCESS Yes
Frontend Build CI (unit tests, linting & sanity checks) frontend-build SUCCESS Yes
Hold Label Check check-hold-label SUCCESS No
Hold Label Check check-hold-label SUCCESS No
License Template Check License Check SUCCESS No
PR Lint lint-check SUCCESS Yes
PR Lint lint-check SUCCESS Yes
Playwright Experimental Tests changes SUCCESS No
Pull Request Labeler labeler SUCCESS No
Python Presto/Hive changes SUCCESS No
Python-Integration changes SUCCESS No
Python-Unit changes SUCCESS No
Superset App CLI tests test-load-examples SUCCESS No
Superset Extensions CLI Package Tests test-superset-extensions-cli-package (current) SUCCESS No
Translations frontend-check-translations SUCCESS No
Validate All GitHub Actions validate-all-ghas SUCCESS No
pre-commit checks pre-commit (current) SUCCESS Yes
supersetbot orglabel based on author superbot-orglabel SUCCESS No
supersetbot orglabel based on author superbot-orglabel SUCCESS No
Frontend Build CI (unit tests, linting & sanity checks) sharded-jest-tests SKIPPED No
Hold Label CI Gate Re-run CI when hold label removed SKIPPED No
Build & publish docker images setup_matrix SUCCESS No
CodeQL Analyze (python) SUCCESS No
Dependency Review python-dependency-liccheck SUCCESS No
E2E cypress-matrix (chrome) SUCCESS No
Playwright Experimental Tests playwright-tests-experimental (chromium) SUCCESS No
Python Presto/Hive test-postgres-presto SUCCESS Yes
Python-Integration test-mysql SUCCESS Yes
Python-Unit event-file SUCCESS No
Superset Extensions CLI Package Tests actions-timeline SUCCESS No
Translations babel-extract SUCCESS No
pre-commit checks actions-timeline SUCCESS No
CodeQL Analyze (javascript) SUCCESS No
Playwright Experimental Tests playwright-tests-experimental (chromium, /app/prefix) SUCCESS No
Frontend Build CI (unit tests, linting & sanity checks) lint-frontend SKIPPED No
Build & publish docker images verify docker build PY_VER override SUCCESS No
E2E playwright-tests (chromium) SUCCESS No
Playwright Experimental Tests actions-timeline SUCCESS No
Python Presto/Hive test-postgres-hive SUCCESS Yes
Python-Integration test-postgres (current) SUCCESS No
Python-Unit unit-tests (current) SUCCESS No
Translations actions-timeline SUCCESS No
E2E playwright-tests (chromium, /app/prefix) SUCCESS No
Build & publish docker images docker-compose-image-tag SKIPPED No
Frontend Build CI (unit tests, linting & sanity checks) validate-frontend SKIPPED No
E2E cypress-matrix-required SUCCESS Yes
Python Presto/Hive actions-timeline SUCCESS No
Python-Integration test-sqlite SUCCESS Yes
Python-Unit unit-tests-required SUCCESS Yes
Frontend Build CI (unit tests, linting & sanity checks) test-storybook SKIPPED No
Build & publish docker images docker-build (superset) SUCCESS No
E2E playwright-tests-required SUCCESS Yes
Python-Integration test-postgres-required SUCCESS Yes
Build & publish docker images docker-build (dev) SUCCESS No
Build & publish docker images docker-build (lean) SUCCESS No
Frontend Build CI (unit tests, linting & sanity checks) bundle-size SKIPPED No
Build & publish docker images actions-timeline SUCCESS No
E2E actions-timeline SUCCESS No
Python-Integration actions-timeline SUCCESS No
Frontend Build CI (unit tests, linting & sanity checks) report-coverage SKIPPED No
Frontend Build CI (unit tests, linting & sanity checks) actions-timeline SUCCESS No
External status Header rules - superset-docs-preview NEUTRAL No
External status Pages changed - superset-docs-preview NEUTRAL No
External status Redirect rules - superset-docs-preview NEUTRAL No
External status CodeQL SUCCESS No
External status zizmor SUCCESS No
Python Presto/Hive Python Unit Test Results SUCCESS No
External status codecov/patch SUCCESS No
External status codecov/project SUCCESS No
External status codecov/project/core-packages-ts SUCCESS No
External status codecov/project/core-packages-tsx SUCCESS No
External status netlify/superset-docs-preview/deploy-preview SUCCESS No

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Maintainer review / landing coordination: @eschutho (requested reviewer and #44283 author), @mikebridge and @richardfogaca (#44317 author/reviewer), could you confirm which baseline repair should land?

#44288 at d6f00dcffec9beb74bc9480b40f38515c5527a5a has all 13 required checks SUCCESS; the full 84-check audit has no failed/pending checks. The #44285 owner reports all 15 failures audited: 12 startup jobs blocked by the two master Alembic heads and three dependent gates. Other driver PR owners have reported the same blocker.

#44317 is another no-op merge of these exact parents, with revision e2f0a912492a, and GitHub reports APPROVED (the recorded approval is on its earlier commit). #44283 also joins these parents inside its ClickHouse migration. Please select only one repair:

Landing parallel merge revisions unchanged recreates multiple heads. No consent, approval of the latest #44317 SHA, or landing decision is assumed. I will notify the #44285 owner only after verifying the selected repair is actually on master; they will then rebase and rerun all applicable CI. No merge or draft conversion will be performed by this session.

abhinav-phi pushed a commit to abhinav-phi/superset that referenced this pull request Sep 15, 2026
`master` currently resolves to two Alembic heads -- c7f53d184ea2 (from
apache#43490) and 88a01c781622 (from apache#43939) -- so every job that runs
`flask db upgrade head` fails on this PR's merge commit with "Multiple
head revisions are present for given argument 'head'": cypress-matrix
(chrome), playwright-tests (chromium, both /app/prefix variants),
playwright-tests-experimental, docker-build (dev) and the matching
*-required gates. This is an upstream breakage on master, not something
the frontend change in this PR can cause or fix; apache#44288 carries the same
remediation.

Add the documented no-op merge revision (see
https://superset.apache.org/docs/contributing/development#merging-db-migrations)
joining both heads so the merge commit resolves to a single head again.

The file is byte-identical to the one in apache#44288 (same revision id
e2f3a1b9c640, same filename, same content) so that whichever PR lands
first the other still merges cleanly, instead of producing a third head
or a "revision present more than once" error. No schema or data changes.
abhinav-phi pushed a commit to abhinav-phi/superset that referenced this pull request Sep 15, 2026
`master` currently resolves to two Alembic heads -- c7f53d184ea2 (from
apache#43490) and 88a01c781622 (from apache#43939) -- so every job that runs
`flask db upgrade head` fails on this PR's merge commit with "Multiple
head revisions are present for given argument 'head'": cypress-matrix
(chrome), playwright-tests (chromium, both /app/prefix variants),
playwright-tests-experimental, docker-build (dev) and the matching
*-required gates. This is an upstream breakage on master, not something
the frontend change in this PR can cause or fix; apache#44288 carries the same
remediation.

Add the documented no-op merge revision (see
https://superset.apache.org/docs/contributing/development#merging-db-migrations)
joining both heads so the merge commit resolves to a single head again.

The file is byte-identical to the one in apache#44288 (same revision id
e2f3a1b9c640, same filename, same content) so that whichever PR lands
first the other still merges cleanly, instead of producing a third head
or a "revision present more than once" error. No schema or data changes.
@mikebridge

Copy link
Copy Markdown
Contributor

let's do this one

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Thanks @mikebridge — recorded your selection of this repair and verified #44317 is closed without merging.

@eschutho, could you complete the requested maintainer review of d6f00dcffec9beb74bc9480b40f38515c5527a5a and confirm #44283 will rebase/reparent onto e2f3a1b9c640 after this lands? #44288 remains REVIEW_REQUIRED; the selection comment is not being treated as formal approval or a merge. All 13 required checks are successful.

Owners of #44284–#44287 remain blocked on actual repaired master. In addition to the CI audit, #44285 reports 135 focused local tests passing and all applicable changed-file pre-commit hooks passing: #44285 (comment) . I will notify all four owners after verifying the maintainer-landed commit is on master with exactly one Alembic head, so they can rebase and rerun every applicable CI check. No merge or draft conversion will be performed by this session.

@mikebridge

Copy link
Copy Markdown
Contributor

Flagging a collision before this lands, because it is cheap to fix now and an emergency afterwards.

There are two open PRs merging the same two alembic heads:

PR revision down_revision
#44288 (this one) e2f3a1b9c640 ("c7f53d184ea2", "88a01c781622")
#44283 f7b3a9c14e02 ("88a01c781622", "c7f53d184ea2")

Same two parents, order swapped. Each consumes both heads and becomes a new childless head, so whichever lands second re-forks master — the very thing both PRs are fixing.

Simulated against a184395eca by AST-walking all 388 revisions in superset/migrations/versions/:

master today         -> 88a01c781622, c7f53d184ea2   (2 heads, broken)
+ #44288 alone       -> e2f3a1b9c640                 (1 head, fixed)
+ #44283 as written  -> e2f3a1b9c640, f7b3a9c14e02   (2 heads, broken again)

The fix is one line in whichever merges second: point its down_revision at the other's revision instead of at the original pair. If this PR lands first, #44283 becomes down_revision = "e2f3a1b9c640".

Also worth knowing for whoever reviews: the enforce-single-migration-head job only runs its assertion when a PR touches superset/migrations/, and reports SUCCESS otherwise so it can stay a required check. That is the right design, but it means the second PR's green checks will not catch this — the two-head state is invisible to every PR that does not touch migrations.

I have no write access here beyond my own PRs, so this needs a committer. @eschutho — you own #44283, so you may be best placed to decide the ordering. Happy to open the one-line re-point against whichever ends up second.

(For the record: I opened a third merge migration myself before spotting this one, and closed it. Flagging so we do not all repair it in parallel.)

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Coordination note found while auditing #44287's CI.

This PR (e2f3a1b9c640) and #44283 (f7b3a9c14e02) both declare:

down_revision = ("88a01c781622", "c7f53d184ea2")

Both merge the same two heads. If both land as currently authored, master goes back to two heads (e2f3a1b9c640 and f7b3a9c14e02) and the Multiple head revisions are present for given argument 'head' startup failure returns repo-wide.

Whichever of the two merges second should be rebased so its down_revision is the single revision that landed first, rather than re-merging the same pair. cc @eschutho for #44283.

Current state confirmed by recomputing the graph from origin/master @ a184395eca7c291d0b8b438f00690c633112f4d7: 387 revisions, 2 heads — c7f53d184ea2, 88a01c781622. This PR is fully green (65 pass / 16 skipped / 0 fail) and awaits committer review; it is the blocker for the migration-dependent jobs on #44285, #44286 and #44287.

@aminghadersohi
aminghadersohi merged commit 4c27cbd into apache:master Sep 15, 2026
84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:db-migration PRs that require a DB migration size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants