Skip to content

feat: opt-in startup migrations for Postgres - #421

Open
lkshrk wants to merge 2 commits into
OpenHands:mainfrom
lkshrk:feat/postgres-auto-migrate
Open

lkshrk wants to merge 2 commits into
OpenHands:mainfrom
lkshrk:feat/postgres-auto-migrate

Conversation

@lkshrk

@lkshrk lkshrk commented Sep 4, 2026

Copy link
Copy Markdown

HUMAN:

Self-hosted deployment on Kubernetes with CloudNativePG Postgres; today I run alembic from a separate Job before the pod starts. Ran tests/test_startup_migrations.py, tests/test_config.py and pre-commit locally on this branch. Stacked on #420.

AGENT:

Stacks on #420 (fix: normalize async Postgres URLs for alembic). Review
and merge that one first; this branch is cut from it, so the diff here is only
the second commit.

Problem

The lifespan runs alembic upgrade head only when the engine is SQLite:

if engine_result.is_sqlite:
    ...
    command.upgrade(alembic_cfg, "head")

A self-hosted deployment that sets AUTOMATION_DB_URL=postgresql+asyncpg://…
therefore starts against an empty database and never creates its schema. Every
scheduler and dispatcher tick logs UndefinedTableError until somebody runs
alembic by hand, and nothing in the service says that is what is required.

Cloud runs its migrations from a separate deploy pipeline, so this cannot simply
be switched on for Postgres — it has to be opt-in.

Fix

A new ServiceSettings field, auto_migrate (AUTOMATION_AUTO_MIGRATE), with
three states:

Value Behaviour
unset (default) SQLite migrates on startup, Postgres does not — exactly today's behaviour
true Migrations run on startup for any backend, Postgres included
false Startup never migrates, even on SQLite
should_migrate = (
    settings.auto_migrate
    if settings.auto_migrate is not None
    else engine_result.is_sqlite
)

Everything else in that block is unchanged: the same migrations-directory
discovery, the same alembic.config.Config, the same failure handling. Only the
two log messages lost their now-inaccurate "SQLite" qualifier.

Concurrent startup is already safe. migrations/env.py takes
pg_advisory_lock(MIGRATION_LOCK_ID) for Postgres and releases it in a
finally, so several pods coming up at once serialize on the lock rather than
racing.

Default off for Postgres means Cloud is untouched — its AUTOMATION_AUTO_MIGRATE
is unset, so the gate evaluates to engine_result.is_sqlite, which is False.

Verification

  • tests/test_startup_migrations.py drives the real lifespan with a stubbed
    engine, stubbed background loops and a mocked alembic.command.upgrade, and
    covers all five combinations: Postgres unset / true / false and SQLite
    unset / false. The opted-in Postgres case also asserts the alembic config
    receives the sync postgresql+pg8000:// URL.
  • Reverting the gate to if engine_result.is_sqlite: fails two of those five,
    so the tests are not vacuous.
  • tests/test_config.py covers the unset default and true/1/false/0
    parsing of AUTOMATION_AUTO_MIGRATE.
  • pre-commit (ruff format, ruff lint, pycodestyle, pyright) clean on the
    changed files.

Docs

The AGENTS.md "Database" table now records the Postgres option and the SQLite
opt-out.

@github-actions github-actions Bot added the type: feat A new feature label Sep 4, 2026
@all-hands-bot

Copy link
Copy Markdown
Contributor

👋 This PR needs a couple of things fixed before OpenHands can review it:

  • the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

Push an update once this is addressed and this check re-runs automatically.

This is an automated check - no AI was used to generate this comment.

lkshrk added 2 commits September 13, 2026 07:58
`AUTOMATION_DB_URL=postgresql+asyncpg://...` is the documented way to point a
self-hosted deployment at Postgres, but `migrations/env.py` handed that URL
straight to the sync `create_engine()`, so `alembic upgrade head` failed with
`sqlalchemy.exc.MissingGreenlet`. Only the `sqlite+aiosqlite` prefix was
rewritten to a sync driver.

Generalize the helper into `normalize_url_for_alembic()`, which also maps
`postgresql+asyncpg` to `postgresql+pg8000` — already a dependency, and already
the driver `env.py` uses for its host/port path — and apply it everywhere a
configured URL reaches alembic. A URL-configured Postgres engine now gets the
same `AUTOMATION_DB_SSL_MODE` connect args as the host/port path.

Claude-Session: https://claude.ai/code/session_01T5LhMv1aKbckQ6SLd4sPGZ
A self-hosted Postgres deployment had no way to get its schema created: the
lifespan ran `alembic upgrade head` only when the engine was SQLite, so a fresh
`AUTOMATION_DB_URL=postgresql+asyncpg://...` deployment logged
`UndefinedTableError` until somebody ran alembic by hand.

Add `AUTOMATION_AUTO_MIGRATE`. Unset keeps today's behaviour — SQLite migrates
on startup, Postgres does not — so Cloud, which migrates from its own deploy
pipeline, is unaffected. `true` also migrates Postgres; `false` disables startup
migrations entirely. Concurrent pods are already safe: `migrations/env.py`
takes `pg_advisory_lock(MIGRATION_LOCK_ID)` for Postgres.

Claude-Session: https://claude.ai/code/session_01T5LhMv1aKbckQ6SLd4sPGZ
@lkshrk
lkshrk force-pushed the feat/postgres-auto-migrate branch from a22a358 to 3aaa792 Compare September 13, 2026 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants