Wire vocab/results schema roles onto CDM models and remove hand-threaded db_schema - #57
nicoloesch wants to merge 33 commits into
Conversation
…mas to tables and cleanup tables not adhering to previously defined standards, adapt CLI to new standard
…es, delegate some functionality to the backends, add the option to reconcile a schema and rectify it if there are stray tables
…per reset and clean-up of test databases, drop the default schema on the primary DB as no longer required
… underlying DB data
…map-vocab-role-wiring
|
|
||
| Parameters | ||
| ---------- | ||
| vocab_engine : sqlalchemy.Engine, optional |
There was a problem hiding this comment.
Route vocabulary CSV loading through vocab_engine, not primary.
Please route inspection, staging, the load session, retries, index/FK handling and sequence reset through vocab_engine end-to-end, and add a live split-engine test asserting rows land only on the vocab target.
| guard no-ops. A role whose connection is test_only=true also | ||
| no-ops, at the guard's own discretion. | ||
| """ | ||
| vocab_engine = vocab_engine if vocab_engine is not None else engine |
There was a problem hiding this comment.
ensure_schema is now correctly split-aware (good), but collect_missing_tables and the existing-table inspector loop still call sa.inspect(engine) only
Please centralise a schema-tag-to-bind lookup and inspect each table on its actual engine before building the plan --> carry the same lookup into reconciliation.
| return Path.cwd() / f"omop-alchemy-backup-{timestamp}{FORMAT_SUFFIXES[backup_format]}" | ||
|
|
||
|
|
||
| def create_database_backup( |
There was a problem hiding this comment.
create_database_backup accepts and records db_schema in its result metadata but never forwards it to backend.prepare_backup, which defaults to the primary role.
| restart_identities=restart_identities, | ||
| cascade=cascade, | ||
| ) | ||
| # One TRUNCATE batch per schema_tag, since truncate_table_batch qualifies its whole list with a single tag. |
There was a problem hiding this comment.
Truncate cross-role foreign keys in one statement per physical connection, not per schema tag. truncate_tables groups the selection by schema_tag and issues one TRUNCATE per group
PostgreSQL's RESTRICT mode requires referencing and referenced tables to be named in the same statement.
Group by physical connection instead of schema tag, and keep the external-blocker check and the "no silent CASCADE" behaviour as-is.
| return tuple(signature) | ||
|
|
||
|
|
||
| def _actual_index_signature(actual_index: ReflectedIndex, backend: Backend) -> tuple[str, ...]: |
There was a problem hiding this comment.
_actual_index_signature only advances the expressions iterator when a column name is None, but PostgreSQL's reflection returns a fully positional expressions list covering every index position.
Please align columns and expressions by shared position.
Summary
Depends on AustralianCancerDataNetwork/orm-loader#39
Requires
pyproject.tomlversion update foroa-configuratorandorm-loaderWhat changed
Role.VOCAB/Role.RESULTSschema tagschema_translate_map's vocab/results keys actually take effect instead of every table silently resolving into the primary schema.backends/no longer threads adb_schemaparameter through 18+ methods by handschema_translate_map.cli_vocab.pyconnect-listener/search_path hack it was compensating for.create_missing_tables()now refuses to create tables under a schema that's silently drifted from what was last recorded, via the schema-provenance-drift protection added inoa-configuratorreconcile_schema()is now genuinely role-aware_ConnContexttrimmed to what it can't already derive from its ownresolvedfieldSupportedDialectenum is gone in favour of the sharedDialectenum fromoa-configuratorinstead, with no change in dispatch behaviour.pg_session's test isolation now resets every non-system schema before and after each test, so a test's own committed DDL/DML in a reserved bookkeeping schema likeMAINTENANCE_SCHEMAno longer depends on some later, unrelated test to clean it up.Schema-drift protection now covers every maintenance command
install_fulltext_columns,manage_indexes, the standaloneindexes clustercommand,truncate_tables, and vocabulary-table creation (cli_vocab.py) are now all wrapped inguard_schema_provenance_forapproximate_row_counts()deleted (dead code, zero callers, superseded by oa-configurator's ownpreview_orphan_schema_tables).physical_schema_of/schema_tagnaming throughout.test_schema_provenance_guard.py: 5 live-Postgres regression tests, one per guarded call siteTesting
Non-default-schema Postgres coverage added throughout, since this whole class of bug is invisible against the default schema.
Checklist
breaking,feature,fix,dependencies, orchore)uv run pytest -q): 370 passeduv run pytest -q -m postgresql): 38 passeduv run ruff check .)