fix: make the migration chain runnable from an empty database
0001 called Base.metadata.create_all(), building the schema from the current models, so on a fresh PostgreSQL 0002 collided on datasets.raw_sha256 and `alembic upgrade head` could never run from zero. 0001 is now an explicit, frozen baseline: the seven base tables exactly as the real history created them, before 0002/0006 extended them. Databases already past 0001 never re-run it. Proof: a fresh `upgrade head` produces a schema identical (pg_dump --schema-only) to the long-lived dev database. 0019 repairs a latent bug found by that comparison: 0006 used server_default="now()" as a string, which PostgreSQL froze at the instant the column was created, so every Agent and Job got the same created_at. It also adds three indexes the models declare but no migration created. central/tests/test_migration_baseline.py guards (via AST) against migrations importing the models or using string timestamp defaults. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,11 +1,99 @@
|
||||
"""initial hub persistence
|
||||
|
||||
Revision ID: 0001
|
||||
|
||||
Frozen, explicit baseline. This revision used to run
|
||||
``Base.metadata.create_all()``, which builds the schema from the *current*
|
||||
models: on an empty database it created tables/columns that 0002..0018 then
|
||||
tried to create again (first collision: ``datasets.raw_sha256`` in 0002), so
|
||||
``alembic upgrade head`` could never run from zero.
|
||||
|
||||
This body reproduces exactly what 0001 created on the databases that went
|
||||
through the real history: the seven base tables, *before* 0002 added
|
||||
``datasets.raw_sha256`` and 0006 added the agent status/timestamp columns.
|
||||
Constraint and index names match the ones those databases already have
|
||||
(PostgreSQL defaults for PK/FK/UNIQUE, ``ix_<table>_<column>`` for indexes).
|
||||
|
||||
Databases already at 0001 or later never re-run this revision, so changing
|
||||
its body only affects new installations. It must never import the ORM
|
||||
models again: a migration describes one point in history, not the present.
|
||||
"""
|
||||
from alembic import op
|
||||
from central.app.models import Base
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision = "0001"
|
||||
down_revision = None
|
||||
branch_labels = depends_on = None
|
||||
def upgrade(): Base.metadata.create_all(op.get_bind())
|
||||
def downgrade(): Base.metadata.drop_all(op.get_bind())
|
||||
|
||||
|
||||
def upgrade():
|
||||
op.create_table(
|
||||
"tenants",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column("name", sa.String(128), nullable=False, unique=True),
|
||||
)
|
||||
op.create_table(
|
||||
"datasets",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column("kind", sa.String(32), nullable=False),
|
||||
sa.Column("provider", sa.String(128), nullable=False),
|
||||
sa.Column("sha256", sa.String(64), nullable=False),
|
||||
sa.Column("version_label", sa.String(256), nullable=False),
|
||||
sa.Column("act", sa.Text(), nullable=False),
|
||||
sa.Column("source_url", sa.Text(), nullable=False),
|
||||
sa.Column("downloaded_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False),
|
||||
sa.Column("status", sa.String(32), nullable=False),
|
||||
sa.Column("record_count", sa.Integer(), nullable=False),
|
||||
sa.UniqueConstraint("provider", "sha256", name="uq_dataset_provider_hash"),
|
||||
)
|
||||
op.create_index("ix_datasets_kind", "datasets", ["kind"])
|
||||
op.create_index("ix_datasets_status", "datasets", ["status"])
|
||||
op.create_table(
|
||||
"ncm_entries",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column("dataset_id", sa.Integer(), sa.ForeignKey("datasets.id"), nullable=False),
|
||||
sa.Column("code", sa.String(16), nullable=False),
|
||||
sa.Column("description", sa.Text(), nullable=False),
|
||||
sa.Column("valid_from", sa.String(32), nullable=False),
|
||||
sa.Column("valid_to", sa.String(32), nullable=False),
|
||||
sa.UniqueConstraint("dataset_id", "code", "valid_from", "valid_to", name="uq_ncm_versioned_entry"),
|
||||
)
|
||||
op.create_index("ix_ncm_entries_dataset_id", "ncm_entries", ["dataset_id"])
|
||||
op.create_index("ix_ncm_entries_code", "ncm_entries", ["code"])
|
||||
op.create_table(
|
||||
"agents",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column("tenant_id", sa.Integer(), sa.ForeignKey("tenants.id"), nullable=False),
|
||||
sa.Column("name", sa.String(128), nullable=False),
|
||||
sa.Column("token_hash", sa.String(64), nullable=False, unique=True),
|
||||
)
|
||||
op.create_index("ix_agents_tenant_id", "agents", ["tenant_id"])
|
||||
op.create_table(
|
||||
"classification_cache",
|
||||
sa.Column("fingerprint", sa.String(64), primary_key=True),
|
||||
sa.Column("tenant_id", sa.Integer(), sa.ForeignKey("tenants.id"), nullable=False),
|
||||
sa.Column("result", sa.JSON(), nullable=False),
|
||||
)
|
||||
op.create_index("ix_classification_cache_tenant_id", "classification_cache", ["tenant_id"])
|
||||
op.create_table(
|
||||
"classification_requests",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column("tenant_id", sa.Integer(), sa.ForeignKey("tenants.id"), nullable=False),
|
||||
sa.Column("agent_id", sa.Integer(), sa.ForeignKey("agents.id"), nullable=False),
|
||||
sa.Column("fingerprint", sa.String(64), nullable=False, unique=True),
|
||||
sa.Column("payload", sa.JSON(), nullable=False),
|
||||
sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False),
|
||||
)
|
||||
op.create_index("ix_classification_requests_tenant_id", "classification_requests", ["tenant_id"])
|
||||
op.create_index("ix_classification_requests_agent_id", "classification_requests", ["agent_id"])
|
||||
op.create_table(
|
||||
"classification_results",
|
||||
sa.Column("id", sa.Integer(), primary_key=True),
|
||||
sa.Column("request_id", sa.Integer(), sa.ForeignKey("classification_requests.id"), nullable=False, unique=True),
|
||||
sa.Column("result", sa.JSON(), nullable=False),
|
||||
)
|
||||
|
||||
|
||||
def downgrade():
|
||||
for table in ("classification_results", "classification_requests", "classification_cache", "agents", "ncm_entries", "datasets", "tenants"):
|
||||
op.drop_table(table)
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
"""Fix frozen timestamp defaults from 0006 and add model indexes never migrated.
|
||||
|
||||
0006 declared ``server_default="now()"`` as a plain string. SQLAlchemy renders
|
||||
that as the literal ``'now()'``, which PostgreSQL casts to a timestamp once,
|
||||
when the column is created — every later row got that same fixed instant
|
||||
instead of the insert time (agents.created_at/updated_at, jobs.created_at).
|
||||
The models rely on the server default, so Jobs and Agents registered after
|
||||
0006 all carried the migration's timestamp. Existing rows are left as they
|
||||
are: their real creation time is not recoverable.
|
||||
|
||||
The models also declare ``index=True`` on three columns that no migration
|
||||
ever indexed.
|
||||
|
||||
Revision ID: 0019_fix_timestamp_defaults
|
||||
Revises: 0018_blocker_installation
|
||||
"""
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision = "0019_fix_timestamp_defaults"
|
||||
down_revision = "0018_blocker_installation"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
_TIMESTAMP_COLUMNS = (("agents", "created_at"), ("agents", "updated_at"), ("jobs", "created_at"))
|
||||
_MISSING_INDEXES = (
|
||||
("ix_kb_observations_tenant_id", "kb_observations", "tenant_id"),
|
||||
("ix_kb_observations_company_key", "kb_observations", "company_key"),
|
||||
("ix_tenant_product_context_company_key", "tenant_product_context", "company_key"),
|
||||
)
|
||||
|
||||
|
||||
def upgrade():
|
||||
for table, column in _TIMESTAMP_COLUMNS:
|
||||
op.alter_column(table, column, server_default=sa.func.now(), existing_type=sa.DateTime(timezone=True), existing_nullable=False)
|
||||
for name, table, column in _MISSING_INDEXES:
|
||||
op.create_index(name, table, [column])
|
||||
|
||||
|
||||
def downgrade():
|
||||
# The frozen-timestamp defaults are a bug and are deliberately not restored.
|
||||
for name, table, _column in reversed(_MISSING_INDEXES):
|
||||
op.drop_index(name, table_name=table)
|
||||
@@ -0,0 +1,64 @@
|
||||
"""Guards for the migration chain being runnable from an empty database.
|
||||
|
||||
0001 used to build the schema from the *current* ORM models, so every later
|
||||
migration collided with it on a fresh database. These checks run offline (no
|
||||
PostgreSQL) and inspect code, not comments, so they fail if that pattern
|
||||
comes back.
|
||||
"""
|
||||
import ast
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
from alembic.config import Config
|
||||
from alembic.script import ScriptDirectory
|
||||
|
||||
ROOT = Path(__file__).parents[2]
|
||||
VERSIONS = ROOT / "central" / "alembic" / "versions"
|
||||
|
||||
|
||||
def _migrations():
|
||||
return {path.name: ast.parse(path.read_text(encoding="utf-8")) for path in sorted(VERSIONS.glob("*.py"))}
|
||||
|
||||
|
||||
def _script_directory():
|
||||
config = Config(str(ROOT / "central" / "alembic.ini"))
|
||||
config.set_main_option("script_location", str(ROOT / "central" / "alembic"))
|
||||
return ScriptDirectory.from_config(config)
|
||||
|
||||
|
||||
def test_no_migration_builds_schema_from_current_models():
|
||||
offenders = []
|
||||
for name, tree in _migrations().items():
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.Attribute) and node.attr in ("create_all", "drop_all"):
|
||||
offenders.append((name, node.attr))
|
||||
if isinstance(node, ast.ImportFrom) and (node.module or "").startswith("central.app"):
|
||||
offenders.append((name, node.module))
|
||||
assert offenders == []
|
||||
|
||||
|
||||
def test_chain_is_linear_with_a_single_head():
|
||||
script = _script_directory()
|
||||
assert len(script.get_heads()) == 1
|
||||
revisions = list(script.walk_revisions())
|
||||
assert revisions[-1].revision == "0001"
|
||||
assert revisions[-1].down_revision is None
|
||||
|
||||
|
||||
def test_timestamp_defaults_are_sql_functions_not_string_literals():
|
||||
# A plain string such as "now()" is rendered as a quoted literal that
|
||||
# PostgreSQL freezes at column creation time. 0006 did that and 0019
|
||||
# repairs it; no other migration may repeat it.
|
||||
offenders = []
|
||||
for name, tree in _migrations().items():
|
||||
if name.startswith("0006_"):
|
||||
continue
|
||||
for node in ast.walk(tree):
|
||||
if (
|
||||
isinstance(node, ast.keyword)
|
||||
and node.arg == "server_default"
|
||||
and isinstance(node.value, ast.Constant)
|
||||
and re.fullmatch(r"\s*(now\s*\(\s*\)|now|current_timestamp|localtimestamp)\s*", str(node.value.value), re.IGNORECASE)
|
||||
):
|
||||
offenders.append(name)
|
||||
assert offenders == []
|
||||
@@ -121,9 +121,15 @@ Agents with `central/scripts/register_agent.py` (see below).
|
||||
`docker exec <postgres-container> pg_dump -U gti gti_fiscal > backup.sql`
|
||||
(or `pg_dumpall` for the whole cluster) on whatever schedule your
|
||||
operations policy requires.
|
||||
- Migrations are additive-only so far (`0001`..`0006`); always run
|
||||
`alembic upgrade head` (the container does this automatically) rather than
|
||||
hand-editing the schema.
|
||||
- Always run `alembic upgrade head` (the container does this automatically)
|
||||
rather than hand-editing the schema. An **empty** database is initialized
|
||||
by that same command — never with `alembic stamp` or `create_all`. `0001` is
|
||||
an explicit, frozen baseline (it used to call `Base.metadata.create_all()`,
|
||||
which made `0002` collide on `datasets.raw_sha256` on any fresh database);
|
||||
`central/tests/test_migration_baseline.py` keeps that from coming back.
|
||||
- `0019` repairs `agents.created_at/updated_at` and `jobs.created_at`, whose
|
||||
defaults `0006` had frozen at the instant it ran (rows created before `0019`
|
||||
keep that frozen value — their real creation time is not recoverable).
|
||||
|
||||
### HTTPS / reverse proxy
|
||||
|
||||
|
||||
Reference in New Issue
Block a user