[Feature]: Write DX-ADR for DB migration- scope and usage of Alembic for SQL schema migrations
- Dominant language
- Python
- Stars
- 19
- Forks
- 45
- Avg merge
- 4d 23h
- Merged PRs (30d)
- 12
Description
## User Story
As a DiracX developer,
I want a short, opinionated document saying what Alembic is and is not responsible for,
So that a developer — including one new to Alembic — can implement it without the scope questions being re-litigated in review, which is what stalled #580.
## Feature Description
DiracX has no schema migration mechanism: `python -m diracx.db init-sql` calls `metadata.create_all`,
which creates missing tables and never alters an existing one. Schema changes to deployed tables ship as
hand-written `ALTER TABLE` statements in PR descriptions (#967) — unversioned, untested, unauditable.
Alembic is the answer we chose (ideally that should be justified in the ADR).
This issue is about writing down the handful of opinionated decisions that a code review cannot make, and then getting out of the way. It should be a short document. Explicitly not a treatment of every migration we might ever need.
The decisions to settle, in rough order of how much they matter:
1. **What is a migration allowed to do?** The initial scope proposed in discussion: migrations that run
**online** on **MySQL and MariaDB**, and that are **safe for all previous supported versions of
DiracX**, since old pods keep serving requests against the new schema during a rolling upgrade.
2. **Can an operator skip a release?** The motivating scenario: release N adds a nullable column and lets
the application fill it in; release N+1 makes it `NOT NULL`. An operator jumping N-1 → N+1 replays both
revisions back to back, with rows still `NULL`, and the second fails. The proposed rule — *a revision
never depends on application code having run; backfills live inside revisions* — plus the preferred
style of `ADD COLUMN ... NOT NULL DEFAULT (...)` in a single step, which sidesteps the dance entirely.
Two wrinkles to record while writing this, both verified:
- `ADD COLUMN Policies JSON NOT NULL DEFAULT '{}'` is **invalid on MySQL** (`ERROR 1101`: JSON columns
can't have a literal default). MySQL needs `DEFAULT (JSON_OBJECT())` / `DEFAULT ('{}')` (8.0.13+);
MariaDB accepts the plain literal. So "MySQL and MariaDB" already needs a dialect branch at the very
first migration.
- A `DEFAULT` is not always right. In #967, `NULL` means "predates the column, recompute the policies",
which is *not* the same as `{}` meaning "has no policies". Defaulting there would silently strip
policy extras from every existing refresh token.
3. **Unit of migration.** One version tree per DiracX SQL database (each `BaseSQLDB` has its own
`MetaData` and its own `DIRACX_DB_URL_`, possibly on a different server), not one global tree.
4. **The boundary.** Alembic owns DDL for databases under `diracx.dbs.sql`. Not the OpenSearch DBs
(`diracx.dbs.os`), not the legacy DIRAC schema, not the Configuration System, and no bulk data
movement.
5. **Who runs it, with what credentials.** Out of band, privileged user, never from the application at
start-up. On the chart side, one simple job at the beginning of the upgrade — no further splitting.
6. **Fresh installs vs existing installs.** `init-sql` becomes `alembic upgrade head`, replaying the chain.
Existing installations get a one-off
`alembic stamp `.
7. **Downgrades.** Implemented for DDL, but production rollback is restore-from-backup.
8. **Autogenerate.** A drafting aid, hand-reviewed, with a CI check enforcing that a revision exists.
9. **Extensions.** How `GubbinsJobDB`, which attaches `GubbinsInfo` to `JobDBBase.metadata`, gets its own
revisions without a DiracX autogenerate proposing `DROP TABLE GubbinsInfo`. This one cannot be
deferred: it breaks Gubbins CI the day `create_all` is retired.
A draft covering all of the above already exists and can be used as the starting point.
## Definition of Done
- [ ] `docs/adr/DX-ADR-003_database_migrations.md` written, following `docs/adr/DX-ADR-XXX_template.md`
- [ ] Added to the table in `docs/adr/index.md`
## Alternatives Considered
- **Skip the ADR and merge an Alembic PR.** This is what #580 attempted; it stalled precisely because the
scope questions surfaced in review with no agreed answer.
- **Merge #967 now, do Alembic later.** Tempting: one nullable column, three affected installations, cheap
to fix if it goes wrong. Rejected because that cheapness is exactly what makes #967 the ideal *first
migration*; merging it manually spends the good test case.
- **Keep `create_all` + `ALTER TABLE` in release notes.** The status quo. Does not survive installations
upgrading independently.
Contributor guide
Assessment
This issue has not been assessed yet.