DIRACGrid / DIRACGrid/diracx

[Feature]: CI guard - fail any PR that changes a schema without a matching migration

Open
#985 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
19
Forks
45
Avg merge
4d 23h
Merged PRs (30d)
12

Description

## User Story

As a DiracX reviewer,
I want CI to fail when a `schema.py` changes and no Alembic revision accompanies it,
So that "I forgot the migration" becomes impossible rather than merely unlikely.

## Feature Description

This is the issue that actually prevents the next #967.

Today a developer can add a column to a `schema.py`, watch the whole test suite go green (fixtures build
every schema from scratch with `create_all`), merge, and break every deployed installation. Nothing in
CI can see the difference between "the model matches the deployed schema" and "the model has drifted".

Once Alembic is in place, `alembic check` answers exactly that question: it autogenerates against a
database migrated to `head` and fails if the diff is non-empty.

**Add a CI job that, for each DiracX SQL database:**

1. Creates an empty MySQL database.
2. Runs `alembic upgrade heads`.
3. Runs `alembic check` against the models.
4. Fails with a message telling the developer to run
`python -m diracx.db revision --db --autogenerate -m "..."`.

**Plus a second job that replays the chain properly:**

- `base → head` then `head → base` (exercising `downgrade()`), against a real MySQL **and** a real
MariaDB — not SQLite. Both are supported production backends, and they already diverge on the very
first migration: MySQL rejects a literal `DEFAULT` on a JSON column and needs
`DEFAULT (JSON_OBJECT())`, while MariaDB accepts `DEFAULT '{}'`. A chain tested on only one of them is
not tested.
- Assert that `upgrade head` on an empty database and `create_all` on an empty database yield identical
reflected metadata — this is what keeps the baseline honest as models evolve.

Note the interaction with the extension work: `alembic check` must be run per branch label with the
right `include_object` filter, otherwise DiracX's check on a gubbins-installed environment reports
`GubbinsInfo` as drift.

## Definition of Done

- [ ] `alembic check` job in CI
- [ ] Failure message names the exact command to fix it
- [ ] Jobs run on every PR touching `diracx-db/` (and on `main`)
- [ ] Extension variant of the check runs in the gubbins CI job with the correct filters
- [ ] A deliberately-broken branch (column added, no revision) is verified to fail CI

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.