rusackas opened a new pull request, #42890:
URL: https://github.com/apache/superset/pull/42890
### SUMMARY
The existing `Check DB migration conflict` workflow is advisory-only: when a
migration lands on the base branch, it scans other *open* PRs that also
touch `superset/migrations/**` and posts a warning comment asking them to
rebase. It never actually computes whether the merge leaves more than one
Alembic head, and nothing about it blocks a merge.
That gap is exactly how master ended up with two divergent heads
(`4f145192b583` and `c4a1b8e2d739`) this week — two unrelated migrations
branched off the same parent and merged independently, and nothing caught
it until `flask db upgrade` broke and #42878 had to land a manual merge
revision to fix it retroactively.
This adds a new job, `enforce_single_migration_head`, to the same workflow
(so it shares the existing `superset/migrations/**` path trigger):
- On `pull_request` it checks out the default merge ref (base + this PR
combined, which is what `actions/checkout` gives you automatically for
`pull_request`-triggered runs), installs Superset with the `base`
requirements set, and runs `superset db heads`. `flask-migrate`'s `heads`
command only walks the migration script directory — it never touches a
real database — so this doesn't need Postgres/Redis services, just a
syntactically valid `SQLALCHEMY_DATABASE_URI` to let the app instantiate.
- If the count isn't exactly 1, the job fails with a pointer to the
merge-revision docs, before the conflicting PR merges rather than after.
- The same job also runs on `push` to `master`/release branches (the
existing trigger already covers this), so if a required-check bypass
ever lets a fork through, master itself fails loudly and immediately
instead of drifting until someone notices a broken upgrade.
The existing advisory-comment job is left as-is; it's still useful context
for the "hey, someone else also touched migrations" case, just no longer
the only line of defense.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — CI workflow only.
### TESTING INSTRUCTIONS
- `zizmor` (GHA security audit, run via pre-commit) passes clean on the
changed file.
- YAML validated with `yaml.safe_load`.
- Reasoned through against `flask_migrate`/`alembic` internals:
`ScriptDirectory.from_config(...).get_heads()`
(what the `heads` command calls) parses migration files directly and
never invokes `env.py`, so it needs no DB connectivity — consistent with
the existing `superset db upgrade` invocation pattern already used in
`superset-app-cli.yml`, just without that job's Postgres/Redis services.
- Once merged, the easiest live test is to open two PRs with migrations
that branch off the same parent and confirm the second one to update
fails this new check.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
**Note for maintainers:** once this job has a green run or two, it should
be added as a required status check in branch protection — that's a repo
settings change I can't make from a PR.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
