From 332b3e0fe4e8a453365560705cea1a3e03c95ed9 Mon Sep 17 00:00:00 2001 From: Donkie Date: Thu, 20 Aug 2026 19:15:24 +0200 Subject: [PATCH] Revert "Give phase 2 somewhere to record a tag's instance identity" This reverts 9aea271. The column bought one thing, that phase 2 would need no migration, and cost something worse: it commits the schema to a shape nobody has run a decoder against. Width, and above all the choice not to make it unique, were read off a design document rather than off working code, and goeland86's codec in #880 computes an effective_instance_uuid that may not mean the same thing. Getting that wrong leaves us altering a populated column on four databases, which is the exact operation the "settle the shape before release" rule exists to avoid. That rule earns its keep for target_type, target_value and filament_id, where retrofitting a discriminator or a nullability change is painful. Appending one nullable column later is not painful, so phase 2 can add it when there is something to write into it. --- .../versions/2026_08_13_1000-fe4970567bb3_tags.py | 5 ----- spoolman/database/models.py | 11 ----------- 2 files changed, 16 deletions(-) diff --git a/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py b/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py index 7e6c732f..2c369be1 100644 --- a/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py +++ b/migrations/versions/2026_08_13_1000-fe4970567bb3_tags.py @@ -39,7 +39,6 @@ def upgrade() -> None: sa.Column("filament_id", sa.Integer(), nullable=True), sa.Column("target_value", sa.String(length=64), nullable=True), sa.Column("format", sa.String(length=32), nullable=True), - sa.Column("instance_uuid", sa.String(length=64), nullable=True), sa.Column("added", sa.DateTime(), nullable=False), sa.ForeignKeyConstraint( ["spool_id"], @@ -60,14 +59,10 @@ def upgrade() -> None: # The whole point of the table: one physical tag means exactly one thing, enforced by # the database rather than by whichever client happened to write last. op.create_index(op.f("ix_tag_uid"), "tag", ["uid"], unique=True) - # Not unique: see models.Tag. A payload copied onto a second sticker can carry one - # instance identity on two rows, both pointing at the same spool. - op.create_index(op.f("ix_tag_instance_uuid"), "tag", ["instance_uuid"], unique=False) def downgrade() -> None: """Perform the downgrade.""" - op.drop_index(op.f("ix_tag_instance_uuid"), table_name="tag") op.drop_index(op.f("ix_tag_uid"), table_name="tag") op.drop_index(op.f("ix_tag_filament_id"), table_name="tag") op.drop_index(op.f("ix_tag_spool_id"), table_name="tag") diff --git a/spoolman/database/models.py b/spoolman/database/models.py index 6f2930f6..0212f404 100644 --- a/spoolman/database/models.py +++ b/spoolman/database/models.py @@ -197,15 +197,4 @@ class Tag(Base): # Free-ish string (openprinttag, ntag, bambu, ...) rather than an enum: informational # in phase 1, and new tag types appear faster than migrations should. format: Mapped[str | None] = mapped_column(String(32)) - # Where phase 2 records a tag's decoded instance identity, so that rescanning a tag - # updates the spool it already created instead of making a second one. Nothing writes - # it yet -- it is here for the same reason the other unused columns are, that the - # table is the expensive half to change once it is populated on four databases, while - # the endpoint that fills it is additive and free. - # - # Indexed for that lookup and deliberately NOT unique. `uid` already enforces that one - # physical tag identifies one thing, and copying a tag's payload onto a second sticker - # (the case in the class docstring) can legitimately put one instance identity on two - # rows that point at the same spool -- which a unique constraint would reject. - instance_uuid: Mapped[str | None] = mapped_column(String(64), index=True) added: Mapped[datetime] = mapped_column()