Schema and data integrity: re-enable the drift check, add natural-key constraints

Offen
  • Stimmt etwas nicht?
#82 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Anfängerfreundlichkeit
35/100
Issue-Typ
Bug
Klarheit
Größtenteils klar
Aktivitätsstatus
Veraltet
Tech-Stack
github-actions, postgresql
Bereich
ci-cd, databases

Rechercherichtung

Beginne mit .github/workflows/_schema_drift.yml und stelle fest, ob die Drift-Prüfung bei einem sauberen Durchlauf erfolgreich ist. Untersuche model/database.py, den Verlauf der Migrationen und tools/find_foilhole_duplicates.py; prüfe die genannten Datenbanktests, bevor du entscheidest, wie doppelte Zeilen behandelt werden sollen. Erledigt bedeutet, dass bestehende Drifts und Duplikate behoben sind, Constraints für natürliche Schlüssel in einer Migration hinzugefügt wurden und CI fehlschlägt, wenn der Drift zurückkehrt.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

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.

Vorherrschende Sprache
TypeScript
Sterne
0
Forks
0
Keine gemergten PRs in 30 T.

Entwicklungsumgebung

Dieses Projekt bietet weder Dev-Container noch Dockerfile noch Beitragsleitfaden – die Einrichtung liegt bei Ihnen. Beginnen Sie mit der README; die allgemeinen Schritte stehen in unserem Leitfaden für den ersten Beitrag.

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus DiamondLightSource/smartem-devtools

Alle Issues in DiamondLightSource/smartem-devtools

Ähnliche Issues

Weitere Issues zu TypeScript

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.