Skip to content

chore: reset alembic migration history with a single initial baseline#572

Merged
fe51 merged 8 commits intomainfrom
clean_alembic
Apr 28, 2026
Merged

chore: reset alembic migration history with a single initial baseline#572
fe51 merged 8 commits intomainfrom
clean_alembic

Conversation

@MateoLostanlen
Copy link
Copy Markdown
Member

Summary

  • Alembic migration history had drifted from the actual DB schema (models changed without matching
    migrations).
  • Collapse all 5 prior migrations into a single initial baseline autogenerated from models.py.
  • Remove SQLModel.metadata.create_all() from init_db so Alembic becomes the sole owner of schema —
    the previous startup path could create missing tables directly from models, which is what let the
    drift happen.
  • Fix the baseline's downgrade() so it also drops the userrole and annotationtype enum types
    (otherwise a re-upgrade fails with CREATE TYPE ... already exists).

How the baseline was generated

Autogenerated against an empty Postgres, so the new migration captures the full current schema as
defined by src/app/models.py. Includes all 10 tables (organizations, webhooks, alerts, cameras,
users, poses, occlusion_masks, sequences, alerts_sequences, detections), their FKs, the
ix_users_login unique index, and the userrole / annotationtype enums. Verified with an upgrade →
downgrade → upgrade round-trip on an empty Postgres.

Deployment — required on every existing environment

The alembic_version table still points to a now-deleted revision, so every existing DB must be
re-stamped before deploying this branch. Run on the VM:

Strip the +asyncpg driver suffix — that form is for SQLAlchemy, not libpq

psql "${POSTGRES_URL/+asyncpg/}" -c "DELETE FROM alembic_version;"
alembic stamp head

No DDL runs — this only updates the version marker. Fresh DBs will get the full schema via the normal
alembic upgrade head path.

Drop SQLModel.metadata.create_all() from init_db — the startup path
used to create missing tables directly from models, which let the DB
drift from the alembic-tracked schema. Alembic now owns all DDL; init_db
only seeds the superadmin organization/user and the default bucket.

Document the one-time alembic_version reset required for databases
migrated under the pre-2026-04 history, so existing volumes do not
fail 'alembic upgrade head' after pulling the new baseline.
…green

Downgrade() previously dropped the tables but left the userrole and
annotationtype enum types behind, so a subsequent alembic upgrade head
would fail on CREATE TYPE ... already exists. Explicitly drop both
types after the table drops. Verified via upgrade -> downgrade -> upgrade
round-trip on an empty postgres.

Also remove the 2026-04-specific restamp section from the migrations
README. It was scoped to a single rollout and has no long-term value
in-repo; the restamp procedure for existing deployments is communicated
via the PR description instead.
@MateoLostanlen MateoLostanlen requested a review from fe51 April 18, 2026 08:29
The dev compose backend command relied on SQLModel.metadata.create_all
in init_db to materialize tables at startup. Now that init_db no longer
creates schema, init_db's SELECTs fail against an empty DB and the
backend healthcheck fails — which is what broke the 'tests' workflow
('container pyronear-backend-1 is unhealthy' at docker compose up
--wait). Match production compose by running alembic first.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.57%. Comparing base (286f9d9) to head (6ce0b16).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #572      +/-   ##
==========================================
+ Coverage   88.34%   88.57%   +0.23%     
==========================================
  Files          51       51              
  Lines        2162     2180      +18     
==========================================
+ Hits         1910     1931      +21     
+ Misses        252      249       -3     
Flag Coverage Δ
backend 88.62% <100.00%> (+0.24%) ⬆️
client 87.80% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 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.

@fe51
Copy link
Copy Markdown
Member

fe51 commented Apr 23, 2026

Hi @MateoLostanlen ! Thank you for the PR !

I have a few questions :

  • Making it work on a empty db is interesting, but I would be glad to see that migrations works fine on a non empty DB (our prod context )
    • Launch dev env et fill the DB
    • Udpates db model
    • Create alembic migration file and run the migration to see if it works as expected
  • Also, the processus for deploying a migration is not clear to me and seems to be done here. Do I miss something ? Does the service need to be stopped ? What are the steps ?

Copy link
Copy Markdown
Collaborator

@Acruve15 Acruve15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much to say on this. My concerns are mostly operational rather than about the migration itself. If I understand correctly, existing environments will crash on the next deployment. Both docker-compose.yml and docker-compose.dev.yml now run alembic upgrade head at startup, and every existing DB has alembic_version pointing at one of the deleted revisions

💡 Can we either land it as a one-off script (e.g. scripts/restamp_alembic.sh?) or add a checklist of enviornements (prod, staging, any dev VMs?) that we tick off before merging?

Another suggestion:

  • CI does not exercise the migration end-to-end. The pytest fixture in src/tests/conftest.py:243 builds the schema with SQLModel.metadata.create_all, so the only path that runs alembic is the compose --wait in tests.yml. Adding a tiny job that runs alembic upgrade head && alembic downgrade base && alembic upgrade head would catch the "CREATE TYPE already exists" class of bug you just fixed.

Comment thread src/migrations/versions/2026_04_18_0805-9700bbccb2f1_initial.py Outdated
@MateoLostanlen
Copy link
Copy Markdown
Member Author

Hi @fe51 ,

I ran the local test — here's the breakdown so you can see exactly what
the deploy process will look like.

Phase 1 — Switch envdev API to clean_alembic without losing data

  1. Build the new image from the clean_alembic branch:
    cd ~/pyronear/api/pyro-api && make build
    This overwrites pyronear/alert-api:latest.

  2. Stop only the api container in envdev (db / minio / frontend stay up):
    cd ~/pyronear/devops/pyro-envdev
    docker compose stop pyro_api && docker compose rm -f pyro_api
    (service name is pyro_api, container_name is api)

  3. Stamp the existing envdev DB at the new initial baseline. This adopts
    the live schema without running any DDL — alembic just records
    "you are at 9700bbccb2f1":
    docker run --rm --network pyro-envdev_default \
    --env-file ~/pyronear/devops/pyro-envdev/.env \
    -e POSTGRES_URL=postgresql+asyncpg://dummy_pg_user:dummy_pg_pwd@db/dummy_pg_db \
    -e SUPERADMIN_ORG=admin \
    pyronear/alert-api:latest \
    alembic stamp head
    Result: alembic_version table created, version_num = 9700bbccb2f1.

  4. Bring api back up with the new image:
    docker compose up -d pyro_api
    Healthy, /status 200, login mateo/mateo works, all data preserved
    (3 orgs, 17 cameras, 198 detections, 21 sequences, 51 poses).

Phase 2 — Apply PR #564's migration on top of the baseline

  1. Check out the PR branch and merge clean_alembic into it so we have
    the new baseline file plus the new migration:
    git fetch origin
    git checkout active_poses
    git merge clean_alembic # auto-merge, no manual conflicts
    The migrations folder ends up with exactly two files:

  2. Rebuild the image with the merged code:
    make build

  3. Run the migration against the live envdev DB:
    docker run --rm --network pyro-envdev_default
    --env-file ~/pyronear/devops/pyro-envdev/.env \
    -e POSTGRES_URL=postgresql+asyncpg://dummy_pg_user:dummy_pg_pwd@db/dummy_pg_db
    -e SUPERADMIN_ORG=admin \
    pyronear/alert-api:latest
    alembic upgrade head
    Output: "Running upgrade 9700bbccb2f1 -> a1b2c3d4e5f6,
    add active column to poses".

  4. Verify schema + backfill:

    • poses.active boolean not null default true is in place
    • 51 / 51 existing poses backfilled to active = true via server_default
    • No other tables touched
  5. Restart the api with the merged image — healthy, login still works.

Result

The envdev DB went from "no alembic at all" to stamped at 9700bbccb2f1
to upgraded to a1b2c3d4e5f6 (head), without losing any data. End-to-end
alembic flow validated on a real production-like DB.

One side note unrelated to this work: GET /api/v1/cameras/ returns 500
for orgs 2 and 3 because MinIO bucket names are prefixed with the api
container's hostname, so a container restart leaves their old buckets
orphaned. Same problem happens on main — independent of the alembic
refactor, can be fixed separately.

For the real deploy, the same recipe applies: build the new image,
stop the api, stamp head once on the existing prod DB, then every
subsequent PR is a normal alembic upgrade head on container start.

Replace sqlmodel.sql.sqltypes.AutoString with sa.String in the baseline
migration so it no longer depends on sqlmodel at load time. Add a
render_item hook in env.py narrowed to AutoString instances so future
autogenerated migrations also emit sa.String(...). Remove import sqlmodel
from script.py.mako so newly generated migration files don't reintroduce
the dependency.
@MateoLostanlen
Copy link
Copy Markdown
Member Author

Thanks @Acruve15 , all your remarks taken into account.

On the AutoString point, pushed in cf345bb:

  • replaced sqlmodel.sql.sqltypes.AutoString with sa.String in the baseline
  • removed import sqlmodel from script.py.mako so future autogenerated
    files stay clean
  • added a render_item hook in env.py narrowed to isinstance(obj, AutoString),
    using obj.impl.length and without injecting a duplicate sa import

On the operational side:

  1. Restamp : I don't think a dedicated script is worth it (true one-shot,
    and the walkthrough above already covers the recipe per env). I'll add
    a per-env checklist (envdev / staging / prod / any local VMs) in the PR
    description and tick them off before merging.
  2. CI round-trip : strong yes. Will land
    alembic upgrade head && alembic downgrade base && alembic upgrade head
    in a follow-up PR right after this one merges, to keep this scope tight.

Add a thin Make wrapper around alembic revision --autogenerate and
alembic upgrade head so contributors do not have to remember the docker
compose exec invocation. Update the migrations README to reflect that
alembic now owns the schema (init_db only seeds the superadmin), list
the cases that --autogenerate misses, and document stamp/current/history
for adopting existing schemas onto the new baseline. Also add a .PHONY
declaration covering every target so the Makefile stays well-behaved if
a file named after a target ever appears in the repo.
Copy link
Copy Markdown
Member

@fe51 fe51 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @MateoLostanlenn thanks a lot for this fresh new start and process to run migration !

@fe51 fe51 merged commit 40e09f0 into main Apr 28, 2026
23 of 24 checks passed
@fe51 fe51 deleted the clean_alembic branch April 28, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants