Conversation
|
|
jlav
marked this pull request as ready for review
September 26, 2026 13:39
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
AGENT:
Why
We're trying to make our systems simpler to deploy. Today the app never migrates its own database, so every install needs one more step to run
alembic upgrade head. That's either a command someone runs by hand, or an init container or separate workload in the deployment.This adds an opt-in flag so the app can safely migrate its database when it starts.
Summary
RUN_MIGRATIONS_ON_STARTUP=true(or1) makes the server runalembic upgrade headin its lifespan before it serves anything. A failed migration stops startup. It's off by default.migrations/env.pyalready takes makes them wait their turn, and the waiters find the database at head.migrations/env.pyis now safe to run inside the app process. It keeps the caller's logging when asked to, and it opens one unpooled connection per run and closes it afterwards. A pooled connection would keep holding the advisory lock and block every other replica.Issue Number
How to Test
This covers the flag, the startup order, an in-process migration of an empty database, the app and a separate alembic process migrating at the same time, and a failing migration.
To see it end to end, point the app at an empty database:
Both workers log
database_migrations_startinganddatabase_migrations_succeeded, each revision runs once, andalembic_versionends at head.Video/Screenshots
N/A
Type
Notes
migrate-dbinit container keeps working as it does today. Installs that turn this flag on can setdatabaseMigrations.migrate: falseand drop it.Enterprise server image for this PR: