DiamondLightSource / DiamondLightSource/smartem-devtools
Schema and data integrity: re-enable the drift check, add natural-key constraints
- Lenguaje dominante
- TypeScript
- Estrellas
- 0
- Forks
- 0
- Métricas de merge de PR
- Sin PR fusionados en 30 d
Descripción
Narrowed. This issue was originally a broad automated "production readiness assessment"
covering eleven areas. Re-verifying its claims against the current codebase, two are true,
actionable, and tracked nowhere else; the rest are stale, overstated, or describe
infrastructure this project does not operate. The discarded items are listed at the bottom
with reasons, so the same assessment does not get re-run and refiled.
## In scope
### 1. The schema drift check is bypassed in CI
`.github/workflows/_schema_drift.yml` still carries `continue-on-error: true`. The job runs and
reports, but cannot fail a build, so drift between the SQLModel definitions and the migration
history can accumulate silently and indefinitely.
This is the more important of the two items, because it is the reason schema problems persist
without being noticed.
Work: establish whether the check currently passes on a clean run; fix whatever it reports; then
remove `continue-on-error` so it is enforced. Re-enabling it before the backlog of drift is
cleared would simply block every PR, so the order matters.
### 2. No unique constraints on natural keys
The following have no unique constraint, despite each being a natural key:
- `foilhole` - `(foilhole_id, gridsquare_uuid)`
- `gridsquare` - `(gridsquare_id, grid_uuid)`
- `micrograph` - `(micrograph_id, foilhole_uuid)`
The only unique constraints in `model/database.py` are on `session_id`, `instruction_id` and
`connection_id`, which are unrelated.
This is not theoretical. `tools/find_foilhole_duplicates.py` exists in the repository, which
means duplicates have been observed and needed hunting.
Work: deduplicate existing data first, then add the constraints in a migration. The
deduplication is the awkward part and needs a decision on which row wins when duplicates differ.
Note the interaction with the agent's filesystem intake: if the ingest path can legitimately
re-observe the same entity, the constraint needs to be paired with an upsert rather than simply
rejecting the second write.
## Explicitly discarded
Re-checked and not carried forward:
- **"Only 13 test files, none for database operations"** - no longer true. There are 32 test
files, including `test_acquisitions.py`, `test_foilholes.py`, `test_gridsquares.py`,
`test_grids.py` and `test_atlases.py`, plus an async database stub in `conftest.py`.
- **"Index created on the wrong table (line 602)"** - unverifiable. The migration it cites is
548 lines long, so that line does not exist. The `qualitypredictionmodelparameter` references
in that file are foreign-key operations, not a misplaced index.
- **"147 indexes causing write overhead"** - that figure counts `create_index` calls across the
whole migration history, including indexes subsequently dropped. It is not a count of live
indexes, and no performance problem attributable to indexing has been observed.
- **"Migration removed CASCADE DELETE"** - overstated. `ondelete="CASCADE"` appears in the same
migration the claim is made about.
- **Backups, WAL archiving, retention and archival** - deferred, and likely not this project's
responsibility. DLS may have a facility-wide strategy; that needs confirming before any work
is scoped here.
- **PostgreSQL configuration tuning, database user separation, row-level security,
table partitioning** - these describe operating our own PostgreSQL instance. In staging and
production, PostgreSQL is a managed service provided to the project. This advice was derived
from the development k3s cluster, where the database is self-hosted, and does not transfer.
- **Connection pool monitoring** - not dismissed on merit, but it belongs with observability
rather than schema integrity, and should be scoped alongside metrics collection generally.
## On the original framing
The "72% production readiness - NOT READY" headline and the "2-3 weeks to production" estimate
were derived from the above, including the items now shown to be false or inapplicable. Neither
figure should be quoted.
Guía de contribución
No hay ninguna guía de contribución indexada para este repositorio
Línea de trabajo
Empieza con .github/workflows/_schema_drift.yml y determina si la comprobación de drift pasa en una ejecución limpia. Inspecciona model/database.py, el historial de migraciones y tools/find_foilhole_duplicates.py; revisa las pruebas de base de datos mencionadas antes de decidir cómo deben tratarse las filas duplicadas. Se considera terminado cuando se hayan resuelto los drifts y duplicados existentes, se hayan añadido restricciones de claves naturales en una migración y CI falle cuando el drift vuelva a aparecer.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- github-actions, postgresql
- Área
- ci-cd, databases
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Estancado
- Claridad
- Bastante claro
- Aptitud para principiantes
- 35/100