test(migrations)+fix: widen drift check to column shape; notifications.created_at NOT NULL (S2 review #2)
The drift check dropped every diff except table/column presence, so a column type or nullability change would slip through. Add modify_type/modify_nullable to the ops it fails on. That immediately surfaced a real pre-existing drift: notifications.created_at was created NULLABLE by migration 0017 but the model has always declared it NOT NULL (Mapped[datetime] + server_default=func.now()). Migration 0066 enforces NOT NULL to match the model — safe, since the server_default has filled every row since day one.
This commit is contained in:
@@ -0,0 +1,24 @@
|
||||
"""notifications.created_at NOT NULL (align schema to the model)
|
||||
|
||||
The model has always declared `Notification.created_at: Mapped[datetime]` (NOT NULL) with
|
||||
`server_default=func.now()`, but migration 0017 created the column without `nullable=False`, so the live
|
||||
schema allowed NULL — a model/schema drift the new migration drift-test (E1 S2) surfaced. The
|
||||
server_default has filled every row since day one, so no NULLs exist; enforcing NOT NULL just makes the
|
||||
schema match the contract the model already promises. Pure DDL, reversible.
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
|
||||
revision: str = "0066_notif_created_at_notnull"
|
||||
down_revision: Union[str, None] = "0065_plex_detail_fields"
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.alter_column("notifications", "created_at", nullable=False)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.alter_column("notifications", "created_at", nullable=True)
|
||||
@@ -9,11 +9,12 @@ Two things the prod boot never checks:
|
||||
|
||||
Both use the `db` fixture, which has already brought the schema to head via `_schema`.
|
||||
|
||||
Scope note: we assert on STRUCTURAL drift only — tables and columns present in the model but not the
|
||||
schema, or vice versa. `compare_metadata` also reports index/constraint/sequence/type differences, but
|
||||
those are dominated by migration-only objects (partial/expression/GIN indexes, CHECK constraints, SERIAL
|
||||
sequences) that can't be expressed on `Base.metadata`, so comparing them wholesale is noise. Missing/extra
|
||||
columns and tables — the "someone changed a model without a migration" case — are the signal worth gating.
|
||||
Scope note: we gate on COLUMN/TABLE drift — a table or column present on one side and not the other, or
|
||||
a column whose type/nullability disagrees. `compare_metadata` also reports index/constraint/sequence
|
||||
differences, but those are dominated by migration-only objects (partial/expression/GIN indexes, CHECK
|
||||
constraints, SERIAL sequences) that can't be expressed on `Base.metadata`, so comparing them wholesale is
|
||||
noise; they're filtered out. The column/table signal is the "someone changed a model without a migration"
|
||||
case worth failing on.
|
||||
"""
|
||||
from alembic import command
|
||||
from alembic.autogenerate import compare_metadata
|
||||
@@ -22,8 +23,11 @@ from alembic.migration import MigrationContext
|
||||
|
||||
from app.db import Base, engine
|
||||
|
||||
# The diff ops that mean a real table/column is present on one side and not the other.
|
||||
_DRIFT_OPS = {"add_table", "remove_table", "add_column", "remove_column"}
|
||||
# The diff ops that mean a real table/column disagreement (presence AND shape) — not the index/
|
||||
# constraint/sequence noise from migration-only objects.
|
||||
_DRIFT_OPS = {
|
||||
"add_table", "remove_table", "add_column", "remove_column", "modify_type", "modify_nullable",
|
||||
}
|
||||
|
||||
|
||||
def _structural_drift():
|
||||
|
||||
Reference in New Issue
Block a user