fix: linearize alembic migration chain to resolve forked 018 revision #181

Merged

forgejo_admin merged 1 commit from fix/179-alembic-forked-chain into main

2026-03-27 04:01:49 +00:00

forgejo_admin commented

2026-03-27 03:56:45 +00:00

Owner

Summary

App is in CrashLoopBackOff because alembic upgrade head fails at revision 018 -- both 019 and 020 had down_revision = "018", creating a fork alembic cannot traverse from a single DB stamp
Linearized the chain so alembic upgrades cleanly from 018 through to current head (024)
Two-file fix, zero data impact -- migrations 019-024 have not been applied to prod yet (DB is stamped at 018)

Changes

alembic/versions/020_add_custom_notes_to_player.py: changed down_revision from "018" to "019", eliminating the fork at 018
alembic/versions/022_merge_heads.py: converted from merge migration (down_revision = ("019", "021")) to linear passthrough (down_revision = "021"), since the fork no longer exists

Test Plan

Tests pass locally -- all 568 tests pass (python -m pytest tests/ -v)
Chain verification -- Python script traced all 24 migrations: single head (024), zero forks, fully linear chain
ruff check and ruff format pass on both modified files
After deploy: kubectl exec -n basketball-api <pod> -- alembic upgrade head succeeds
After deploy: SELECT version_num FROM alembic_version returns 024
After deploy: pod reaches 1/1 Running without CrashLoopBackOff

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #179
Blocked PRs: #170 (jersey sync), #173 (teams/save fix), #171 (Baby Betty data fix)

## Summary - App is in CrashLoopBackOff because `alembic upgrade head` fails at revision 018 -- both 019 and 020 had `down_revision = "018"`, creating a fork alembic cannot traverse from a single DB stamp - Linearized the chain so alembic upgrades cleanly from 018 through to current head (024) - Two-file fix, zero data impact -- migrations 019-024 have not been applied to prod yet (DB is stamped at 018) ## Changes - `alembic/versions/020_add_custom_notes_to_player.py`: changed `down_revision` from `"018"` to `"019"`, eliminating the fork at 018 - `alembic/versions/022_merge_heads.py`: converted from merge migration (`down_revision = ("019", "021")`) to linear passthrough (`down_revision = "021"`), since the fork no longer exists ## Test Plan - [x] Tests pass locally -- all 568 tests pass (`python -m pytest tests/ -v`) - [x] Chain verification -- Python script traced all 24 migrations: single head (024), zero forks, fully linear chain - [x] `ruff check` and `ruff format` pass on both modified files - [ ] After deploy: `kubectl exec -n basketball-api <pod> -- alembic upgrade head` succeeds - [ ] After deploy: `SELECT version_num FROM alembic_version` returns `024` - [ ] After deploy: pod reaches `1/1 Running` without CrashLoopBackOff ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #179 - Blocked PRs: #170 (jersey sync), #173 (teams/save fix), #171 (Baby Betty data fix)

forgejo_admin added 1 commit

2026-03-27 03:56:45 +00:00

fix: linearize alembic migration chain to resolve forked 018 revision

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

60a86ead0c

The migration chain forked at revision 018 — both 019 and 020 had
down_revision="018", causing alembic upgrade head to fail when the DB
was stamped at 018. This put the app into CrashLoopBackOff.

Fix: change 020's down_revision from "018" to "019" to make the chain
linear (018 -> 019 -> 020 -> 021 -> 022 -> 023 -> 024), and convert
022 from a merge migration to a simple linear passthrough.

Closes #179

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-27 03:57:10 +00:00

Author

Owner

Review: LGTM

Diff scope: 2 files, +9/-9 lines. Migration-only change, zero application code touched.

Verification

Chain linearity: Confirmed via script -- 24 migrations, single head (024), zero forks, fully linear: 001 -> ... -> 018 -> 019 -> 020 -> 021 -> 022 -> 023 -> 024
Safety: Migrations 019-024 have NOT been applied to prod (DB stamped at 018), so rewriting down_revision values is safe -- alembic will traverse the new linear chain on next upgrade head
e09c9e678004 untouched: Correctly left alone -- it is part of the main chain (005 -> e09c9e678004 -> 007)
022 retained: Kept as no-op passthrough to preserve revision numbering continuity rather than deleting and renumbering
Linting: ruff check + format pass on both files
Tests: 568/568 pass

Findings

No issues found. Minimal, correct fix for a live outage.

## Review: LGTM **Diff scope**: 2 files, +9/-9 lines. Migration-only change, zero application code touched. ### Verification - **Chain linearity**: Confirmed via script -- 24 migrations, single head (024), zero forks, fully linear: `001 -> ... -> 018 -> 019 -> 020 -> 021 -> 022 -> 023 -> 024` - **Safety**: Migrations 019-024 have NOT been applied to prod (DB stamped at 018), so rewriting `down_revision` values is safe -- alembic will traverse the new linear chain on next `upgrade head` - **e09c9e678004 untouched**: Correctly left alone -- it is part of the main chain (`005 -> e09c9e678004 -> 007`) - **022 retained**: Kept as no-op passthrough to preserve revision numbering continuity rather than deleting and renumbering - **Linting**: ruff check + format pass on both files - **Tests**: 568/568 pass ### Findings No issues found. Minimal, correct fix for a live outage.

forgejo_admin commented

2026-03-27 03:58:47 +00:00

Author

Owner

PR #181 Review

DOMAIN REVIEW

Tech stack: Python / Alembic (SQLAlchemy migrations)

Critical verification -- migration chain linearity:

I traced every down_revision from 001 through 024. The full chain post-fix is:

001 -> 002 -> 003 -> 004 -> 005 -> e09c9e678004 -> 007 -> 008 -> 009 -> 010
-> 011 -> 012 -> 013 -> 014 -> 015 -> 016 -> 017 -> 018 -> 019 -> 020 -> 021
-> 022 -> 023 -> 024

Single head at 024. Zero forks. Fully linear. Verified by reading every migration file's down_revision value.

Critical verification -- e09c9e678004 untouched:

e09c9e678004_add_division_column_to_players.py is NOT in the PR diff. File contents confirmed unchanged (down_revision = '005', revision = 'e09c9e678004'). No catastrophic risk.

The fix itself:

File	Before	After	Correct?
`020_add_custom_notes_to_player.py`	`down_revision = "018"`	`down_revision = "019"`	Yes -- eliminates fork at 018
`022_merge_heads.py`	`down_revision = ("019", "021")`	`down_revision = "021"`	Yes -- merge migration no longer needed since chain is linear

The 022 migration is correctly converted to a no-op passthrough with pass in both upgrade() and downgrade(). This preserves revision numbering continuity without introducing any schema changes.

Docstring accuracy: The 022 file's docstring accurately describes the linearized chain: 018 -> 019 -> 020 -> 021 -> 022. This matches what I verified in the files.

Import ordering in 022: The PR swaps the import order from from alembic import op; import sqlalchemy as sa to import sqlalchemy as sa; from alembic import op. This follows PEP 8 (stdlib-style before third-party, import before from). Both have # noqa: F401 since they are unused in the passthrough -- correct.

Data safety: PR body correctly notes that migrations 019-024 have not been applied to prod (DB stamped at 018). This means the down_revision changes have zero data impact -- alembic will simply traverse the corrected chain on next upgrade head.

BLOCKERS

None.

This is a metadata-only fix (migration chain pointers). No new functionality is added, so no new test coverage is required. The existing 568 tests passing confirms no regression. The chain verification script described in the Test Plan is the appropriate validation for this type of fix.

NITS

None. The diff is minimal and surgical -- exactly what a critical production fix should look like.

SOP COMPLIANCE

Branch named after issue (fix/179-alembic-forked-chain references #179)
PR body follows template (Summary, Changes, Test Plan, Related)
Related references parent issue (Closes #179)
No plan slug needed (board-driven bug fix, not plan work)
No secrets committed
No unnecessary file changes (2 files, both directly related to the fix)
Commit messages are descriptive
Scope is tight -- no feature creep

PROCESS OBSERVATIONS

DORA impact: This is a MTTR (Mean Time to Recovery) event. The app is in CrashLoopBackOff -- a live outage. The fix is correctly scoped to the minimal change needed: two down_revision pointer corrections. No scope creep into "while we're here" improvements.

Blocked PRs: PR body correctly identifies three PRs blocked by this fix (#170, #173, #171). Merging this unblocks the deployment pipeline for those features.

Previous attempt: PR #169 (closed) was an earlier fix for the same class of issue (#166). The fact that this recurred as #179 suggests the original fix (merge migration approach in 022) was a band-aid rather than a root cause fix. This PR correctly addresses the root cause by linearizing the chain instead of merging forks.

Post-deploy verification: The Test Plan includes the right post-deploy checks (alembic upgrade head, version table query, pod health). These should be executed immediately after merge.

VERDICT: APPROVED

## PR #181 Review ### DOMAIN REVIEW **Tech stack**: Python / Alembic (SQLAlchemy migrations) **Critical verification -- migration chain linearity:** I traced every `down_revision` from 001 through 024. The full chain post-fix is: ``` 001 -> 002 -> 003 -> 004 -> 005 -> e09c9e678004 -> 007 -> 008 -> 009 -> 010 -> 011 -> 012 -> 013 -> 014 -> 015 -> 016 -> 017 -> 018 -> 019 -> 020 -> 021 -> 022 -> 023 -> 024 ``` Single head at 024. Zero forks. Fully linear. Verified by reading every migration file's `down_revision` value. **Critical verification -- e09c9e678004 untouched:** `e09c9e678004_add_division_column_to_players.py` is NOT in the PR diff. File contents confirmed unchanged (`down_revision = '005'`, `revision = 'e09c9e678004'`). No catastrophic risk. **The fix itself:** | File | Before | After | Correct? | |------|--------|-------|----------| | `020_add_custom_notes_to_player.py` | `down_revision = "018"` | `down_revision = "019"` | Yes -- eliminates fork at 018 | | `022_merge_heads.py` | `down_revision = ("019", "021")` | `down_revision = "021"` | Yes -- merge migration no longer needed since chain is linear | The 022 migration is correctly converted to a no-op passthrough with `pass` in both `upgrade()` and `downgrade()`. This preserves revision numbering continuity without introducing any schema changes. **Docstring accuracy:** The 022 file's docstring accurately describes the linearized chain: `018 -> 019 -> 020 -> 021 -> 022`. This matches what I verified in the files. **Import ordering in 022:** The PR swaps the import order from `from alembic import op; import sqlalchemy as sa` to `import sqlalchemy as sa; from alembic import op`. This follows PEP 8 (stdlib-style before third-party, `import` before `from`). Both have `# noqa: F401` since they are unused in the passthrough -- correct. **Data safety:** PR body correctly notes that migrations 019-024 have not been applied to prod (DB stamped at 018). This means the `down_revision` changes have zero data impact -- alembic will simply traverse the corrected chain on next `upgrade head`. ### BLOCKERS None. This is a metadata-only fix (migration chain pointers). No new functionality is added, so no new test coverage is required. The existing 568 tests passing confirms no regression. The chain verification script described in the Test Plan is the appropriate validation for this type of fix. ### NITS None. The diff is minimal and surgical -- exactly what a critical production fix should look like. ### SOP COMPLIANCE - [x] Branch named after issue (`fix/179-alembic-forked-chain` references #179) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related references parent issue (`Closes #179`) - [x] No plan slug needed (board-driven bug fix, not plan work) - [x] No secrets committed - [x] No unnecessary file changes (2 files, both directly related to the fix) - [x] Commit messages are descriptive - [x] Scope is tight -- no feature creep ### PROCESS OBSERVATIONS **DORA impact:** This is a MTTR (Mean Time to Recovery) event. The app is in CrashLoopBackOff -- a live outage. The fix is correctly scoped to the minimal change needed: two `down_revision` pointer corrections. No scope creep into "while we're here" improvements. **Blocked PRs:** PR body correctly identifies three PRs blocked by this fix (#170, #173, #171). Merging this unblocks the deployment pipeline for those features. **Previous attempt:** PR #169 (closed) was an earlier fix for the same class of issue (#166). The fact that this recurred as #179 suggests the original fix (merge migration approach in 022) was a band-aid rather than a root cause fix. This PR correctly addresses the root cause by linearizing the chain instead of merging forks. **Post-deploy verification:** The Test Plan includes the right post-deploy checks (`alembic upgrade head`, version table query, pod health). These should be executed immediately after merge. ### VERDICT: APPROVED