db: collapse alembic 0001..0089 into one baseline (#3266)
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 3s
Build images / build-agent (push) Successful in 8s
CI / frontend-build (push) Successful in 17s
CI / backend-lint-and-test (push) Successful in 30s
Build images / build-web (push) Successful in 2m12s
Build images / build-ml (push) Successful in 2m52s
CI / integration (push) Failing after 3m42s
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 3s
Build images / build-agent (push) Successful in 8s
CI / frontend-build (push) Successful in 17s
CI / backend-lint-and-test (push) Successful in 30s
Build images / build-web (push) Successful in 2m12s
Build images / build-ml (push) Successful in 2m52s
CI / integration (push) Failing after 3m42s
89 files and 6,300 lines become one file of 807. Nothing about the resulting schema changes; what goes away is the requirement that a new installation replay our development history to arrive at it. revision = "0089", down_revision = None. That pairing IS the migration strategy for existing installs, not a detail of it: a deployed database already has alembic_version = '0089' from running the real 0089, so alembic reads the version table, sees head reached, and does nothing. No stamp is required — which matters, because `alembic stamp` writes a version string without validating anything about the schema it is writing it against, and a wrong stamp is indistinguishable from a right one until the next migration fails. An empty database runs the file and records 0089. Both paths converge. The next migration is 0090, as it would have been; the numbering is continuous across the collapse on purpose. Autogenerate produced nearly all of this unaided, which was NOT true of the first attempt — that one was reverted because the generator silently dropped eleven indexes and three uniqueness guarantees. #3275 put those on the models first, so the HNSW index with its opclass, the COALESCE expression index, the partial uniques, 107 server_defaults and the enum CHECKs are all emitted now. Doing the reconciliation before the squash, rather than after, is what made this work. Hand-added, because none of it can live in a model: * CREATE EXTENSION vector / tsm_system_rows (0001, 0004) — database objects, not table metadata. * The pgvector import. Autogenerate writes qualified pgvector.sqlalchemy.vector.VECTOR references without importing the package, so its own output cannot run (run 4988). * THE TWO SEED ROWS. 0002 and 0003 did not only build schema — each inserted a settings singleton, and nothing in the app ever creates them: ImportSettings.load() and MLSettings.load() are select(...).scalar_one(), which RAISES NoResultFound rather than returning None. A models-only baseline would leave both tables empty and crash a fresh install on first settings access, while baseline.yml reported a perfect schema match. Only running the app against a new database finds that. Not carried over: 0023's DELETE FROM tag and 0047's series deletes, which are historical cleanups operating on rows an empty database lacks. downgrade() raises. A baseline's downgrade is "drop every table", which is a data-loss event wearing a migration as a disguise; offering it as one invites someone to run it. Restore from a backup. Also removed, per the plan: the 10 test_migration_*.py files (they assert intermediate states and backfills that no longer exist — a test that a column exists is already the model tests' job) and backend/app/utils/artist_backfill.py, whose only importer was 0008. Verified no other consumer anywhere in backend/ or tests/. baseline.yml changes with it. chain_ref now DEFAULTS to725bf15, since the tree no longer carries a chain to compare against — that pinned commit is the last one that does. And the CheckConstraint repair is removed, because it never fired. I added it claiming autogenerate re-doubles a constraint name on the round trip and asserted inb979062that it was "still correct and still needed". It is not: autogenerate wraps names in op.f(), which marks them already-formatted and blocks the convention from re-applying. Tested against the real candidate line — the regex matches nothing. What actually fixed the mismatch was 0088's renames alone. The doubling is a real hazard, but of hand-writing a pre-prefixed name, not of the generator; the comment asserting otherwise was worse than the dead code under it. The header comment is rewritten for the same reason — it described 87 revisions, and claimed the HNSW index could not be expressed in a model, which #3275 disproved. It now also states plainly what this check CANNOT see: it compares schema, so a green run means the schema is right, not that the baseline is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017QHszn9H8VBvx5Ke8x1hvw
This commit is contained in:
@@ -1,42 +1,46 @@
|
||||
|
||||
# TEMPORARY — milestone 328 steps 1-2. Delete once the baseline is stamped.
|
||||
# TEMPORARY — milestone 328. Delete once the baseline has shipped and settled.
|
||||
#
|
||||
# Squashing 87 alembic revisions into one baseline has exactly one dangerous
|
||||
# failure: the generated baseline does not reproduce the schema the chain
|
||||
# produced, `alembic stamp` writes a version string anyway (it validates
|
||||
# NOTHING), and the divergence surfaces on the next real migration against the
|
||||
# operator's live data.
|
||||
# Collapsing 89 alembic revisions into one baseline has exactly one dangerous
|
||||
# failure: the baseline does not reproduce the schema the chain produced, and
|
||||
# the divergence surfaces later, on the operator's live data, in whatever
|
||||
# migration comes next.
|
||||
#
|
||||
# So this workflow does the comparison in CI, where a pgvector Postgres already
|
||||
# gets built from the chain on every integration run, and nothing is at risk.
|
||||
# It answers one question: does `upgrade head` on the collapsed chain produce a
|
||||
# byte-identical schema to `upgrade head` on the 87-revision chain?
|
||||
# So the comparison happens in CI, against a throwaway pgvector Postgres, where
|
||||
# nothing is at risk. It answers one question: does `upgrade head` on the
|
||||
# collapsed tree produce the same schema as `upgrade head` on the full chain?
|
||||
#
|
||||
# The chain is read from git rather than from the working tree, so this keeps
|
||||
# working AFTER the old revisions are deleted — `chain_ref` names a commit that
|
||||
# still has them. That is what makes this the proof for step 1 and the
|
||||
# pre-flight for step 2, rather than a one-shot script.
|
||||
# The chain is read out of GIT, not the working tree, which is what lets this
|
||||
# keep working now that the revisions are deleted — `chain_ref` names a commit
|
||||
# that still carries 0001..0089. That is the whole reason this is a workflow
|
||||
# rather than a script someone ran once.
|
||||
#
|
||||
# While the chain is still present it also autogenerates a candidate baseline
|
||||
# from the models and prints it. That is a starting point, NOT the answer:
|
||||
# autogenerate reads SQLAlchemy metadata, and three things here do not live
|
||||
# there —
|
||||
# * CREATE EXTENSION vector (0001)
|
||||
# * CREATE EXTENSION tsm_system_rows (0004)
|
||||
# * the HNSW index on image_record.siglip_embedding, which is raw SQL
|
||||
# because alembic's create_index cannot express `USING hnsw (...)` (0036)
|
||||
# plus any CHECK constraint or server_default that a migration added without
|
||||
# the model declaring it. Those must be hand-added, and the diff below is what
|
||||
# proves none were missed.
|
||||
# WHAT THIS CANNOT SEE, and it matters: the comparison is of SCHEMA. Migrations
|
||||
# 0002 and 0003 also INSERTED rows (the import_settings and ml_settings
|
||||
# singletons), and the application reads those with scalar_one(), which raises
|
||||
# on an empty result. A baseline that omitted them would produce an identical
|
||||
# schema, pass this check with a perfect diff, and crash a fresh install on its
|
||||
# first settings access. Only running the app against a new database finds
|
||||
# that class of defect. Do not read a green run here as "the baseline is
|
||||
# correct" — read it as "the schema is correct".
|
||||
#
|
||||
# Autogenerate now emits nearly all of the baseline unaided, which was NOT true
|
||||
# before #3275 put the previously migration-only objects onto the models — the
|
||||
# HNSW index with its opclass, the COALESCE expression index, the partial
|
||||
# unique indexes, 107 server_defaults, the enum CHECKs. An earlier attempt at
|
||||
# this squash was reverted precisely because the generator dropped them all
|
||||
# silently. What still needs hand-adding is only what cannot live in a model:
|
||||
# the two CREATE EXTENSION statements, the two seed rows, and the pgvector
|
||||
# import the generator forgets to write.
|
||||
name: Alembic baseline
|
||||
|
||||
on:
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
chain_ref:
|
||||
description: 'Commit/tag carrying the full chain; blank = this ref (use a pinned commit only AFTER the collapse)'
|
||||
description: 'Commit/tag carrying the full 0001..0089 chain (pinned: the tree no longer has it)'
|
||||
type: string
|
||||
default: ''
|
||||
default: '725bf15'
|
||||
mode:
|
||||
description: 'chain = compare against this tree''s migrations; models = compare against a schema built from the MODELS'
|
||||
type: string
|
||||
@@ -222,42 +226,6 @@ jobs:
|
||||
# comparison is about whether the models describe the schema.
|
||||
sed -i '0,/^import sqlalchemy as sa$/s//import sqlalchemy as sa\nimport pgvector.sqlalchemy.vector/' alembic/versions/*.py
|
||||
grep -n 'import pgvector' alembic/versions/*.py
|
||||
# Second generator defect, same class as the missing import.
|
||||
#
|
||||
# base.py's naming convention includes %(constraint_name)s for ck,
|
||||
# which — unlike uq/fk/ix — means the convention is applied even to
|
||||
# a CheckConstraint that HAS a name. So a model declaring
|
||||
# name="singleton" correctly becomes ck_ml_settings_singleton in
|
||||
# the metadata. Autogenerate then writes that RENDERED name into
|
||||
# the migration, and running the migration applies the convention a
|
||||
# SECOND time: ck_ml_settings_ck_ml_settings_singleton.
|
||||
#
|
||||
# That is round-tripping damage done by the generator, not a claim
|
||||
# the models make, so it is repaired here rather than counted as a
|
||||
# schema difference. Undone by removing the ck_<table>_ prefix the
|
||||
# convention will re-add — the exact inverse, and it only fires on
|
||||
# a name that actually carries its own table's prefix.
|
||||
python3 - alembic/versions/*.py <<'PYEOF'
|
||||
import re, sys
|
||||
|
||||
table = None
|
||||
for path in sys.argv[1:]:
|
||||
out = []
|
||||
for line in open(path):
|
||||
m = re.search(r"op\.create_table\(\s*[\"']([A-Za-z0-9_]+)[\"']", line)
|
||||
if m:
|
||||
table = m.group(1)
|
||||
if table and "CheckConstraint" in line:
|
||||
prefix = f"ck_{table}_"
|
||||
line = re.sub(
|
||||
r"(name=[\"'])" + re.escape(prefix),
|
||||
r"\1",
|
||||
line,
|
||||
)
|
||||
out.append(line)
|
||||
open(path, "w").writelines(out)
|
||||
PYEOF
|
||||
grep -n 'CheckConstraint' alembic/versions/*.py || true
|
||||
ls alembic/versions/*.py
|
||||
DB_NAME=fc_base alembic upgrade head
|
||||
rm -f alembic/versions/*.py
|
||||
|
||||
Reference in New Issue
Block a user