Let a baseline meet a table that is already there
Some checks failed
CI / backend (push) Failing after 11s
CI / naming (push) Successful in 2s
CI / frontend (push) Successful in 11s
CI / migrations-mysql (push) Failing after 7s

Reported: `flask plugin upgrade-all` failed the tools plugin with "table
securesendsecrets already exists".

Every post-cutover baseline emitted a plain op.create_table. That is correct
only while a table's existence and its plugin's Alembic stamp can never
disagree, and they can. Anything that builds tables from the models -
db.create_all(), which `flask db-utils create-all` calls - creates them and
stamps nothing. A run that creates a table and then fails before its stamp
commits leaves the same split, and re-running is the obvious thing to try. In
both cases upgrade-all meets existing tables with an unstamped chain and the
create is fatal.

Nothing caught it because the migration test DROPS the post-cutover tables
db.create_all() built before running upgrade-all. That arranges the one state
where an unguarded create works, and never the state the report came from. The
new test is the same sequence with the drop removed for the plugin under test.

create_table_if_missing and create_index_if_missing are in alembic_template.py
next to create_plugin_tables, which had the idempotency rule right and turns out
to be called by nobody - its docstring claimed every 0001 used it. Fixed the
docstring rather than rewiring the baselines onto it, because the reason they
hand-write their DDL still stands: the helper's per-plugin MetaData cannot
resolve a foreign key to a core table at CreateTable-compile time.

INDEXES ARE MATCHED ON COLUMNS, NOT NAME, and that distinction matters. The two
creators disagree about names - db.create_all() takes SQLAlchemy's default
convention (ix_securesendsecrets_token) and a baseline spells it the way this
repo's naming rules require (ixsecuresendsecretstoken). A name-keyed guard would
have called the index missing and added a second one over the same column, which
MySQL permits with a warning, so every affected site would have quietly grown
duplicates. The test asserts no table ends up with two indexes over the same
columns.

Applied to all nine table-creating plugin migrations, not just the tools
baseline that was reported: the four later migrations that create tables have
the same exposure, and geenforce, measuringtools, printedparts and backups are
the same defect waiting for a different site.

WHAT THIS DOES NOT FIX. It makes a baseline idempotent, not a whole chain. A
longer chain can still meet an ALTER whose column is already present -
printedparts adding gagelabtag to a table create_all built at head is the case
that showed up while testing. Guarding every op one at a time is the wrong
answer to that; a database built from the models is already AT head and its
chains want stamping rather than replaying, and there is no `flask plugin stamp`
today. Left alone deliberately rather than half-built, and the new test is
scoped to single-baseline plugins so it does not pretend otherwise.
This commit is contained in:
cproudlock
2026-08-24 09:28:37 -04:00
parent de3c5aa562
commit b96637e1a1
12 changed files with 285 additions and 74 deletions

View File

@@ -12,6 +12,11 @@ time (NoReferencedTableError). Same shape autogenerate produces. Tables inherit
the connection's default charset, matching how the core chain creates its own.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -23,7 +28,7 @@ depends_on = None
def upgrade():
op.create_table(
create_table_if_missing(
'backuprevisions',
sa.Column('backuprevisionid', sa.Integer(), nullable=False),
sa.Column('assetid', sa.Integer(), nullable=False),
@@ -42,15 +47,15 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('backuprevisionid'),
)
op.create_index('ixbackuprevisionsassetid', 'backuprevisions', ['assetid'])
op.create_index('ixbackuprevisionsbackupkind', 'backuprevisions',
['backupkind'])
op.create_index('ixbackuprevisionscontenthash', 'backuprevisions',
['contenthash'])
create_index_if_missing('ixbackuprevisionsassetid', 'backuprevisions', ['assetid'])
create_index_if_missing('ixbackuprevisionsbackupkind', 'backuprevisions',
['backupkind'])
create_index_if_missing('ixbackuprevisionscontenthash', 'backuprevisions',
['contenthash'])
# The dedup read path is "latest revision for this asset+kind", so the
# composite index is the one that actually gets used on every collector post.
op.create_index('ixbackuprevisionsassetkind', 'backuprevisions',
['assetid', 'backupkind'])
create_index_if_missing('ixbackuprevisionsassetkind', 'backuprevisions',
['assetid', 'backupkind'])
def downgrade():

View File

@@ -8,6 +8,11 @@ tables are created parent-first. computertypeid / measuringtooltypeid /
publishedby are SOFT references (plain columns, no FK) to optional plugins/core.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -19,7 +24,7 @@ depends_on = None
def upgrade():
op.create_table(
create_table_if_missing(
'manifestscopes',
sa.Column('scopeid', sa.Integer(), nullable=False),
sa.Column('scopename', sa.String(length=64), nullable=False),
@@ -37,7 +42,7 @@ def upgrade():
sa.PrimaryKeyConstraint('scopeid'),
sa.UniqueConstraint('scopename', 'phase', name='uq_scope_name_phase'),
)
op.create_table(
create_table_if_missing(
'manifestentries',
sa.Column('entryid', sa.Integer(), nullable=False),
sa.Column('scopeid', sa.Integer(), nullable=False),
@@ -80,10 +85,10 @@ def upgrade():
sa.PrimaryKeyConstraint('entryid'),
sa.UniqueConstraint('scopeid', 'name', name='uq_entry_scope_name'),
)
op.create_index('idx_entry_scope', 'manifestentries', ['scopeid'])
op.create_index('idx_entry_scope_order', 'manifestentries',
['scopeid', 'sortorder'])
op.create_table(
create_index_if_missing('idx_entry_scope', 'manifestentries', ['scopeid'])
create_index_if_missing('idx_entry_scope_order', 'manifestentries',
['scopeid', 'sortorder'])
create_table_if_missing(
'manifestentrypctypes',
sa.Column('entrypctypeid', sa.Integer(), nullable=False),
sa.Column('entryid', sa.Integer(), nullable=False),
@@ -93,8 +98,8 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('entrypctypeid'),
)
op.create_index('idx_pctype_entry', 'manifestentrypctypes', ['entryid'])
op.create_table(
create_index_if_missing('idx_pctype_entry', 'manifestentrypctypes', ['entryid'])
create_table_if_missing(
'manifestentryhostnames',
sa.Column('entryhostnameid', sa.Integer(), nullable=False),
sa.Column('entryid', sa.Integer(), nullable=False),
@@ -104,8 +109,8 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('entryhostnameid'),
)
op.create_index('idx_hostname_entry', 'manifestentryhostnames', ['entryid'])
op.create_table(
create_index_if_missing('idx_hostname_entry', 'manifestentryhostnames', ['entryid'])
create_table_if_missing(
'manifestentrymachinenumbers',
sa.Column('entrymachinenumberid', sa.Integer(), nullable=False),
sa.Column('entryid', sa.Integer(), nullable=False),
@@ -115,9 +120,9 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('entrymachinenumberid'),
)
op.create_index('idx_machinenumber_entry', 'manifestentrymachinenumbers',
['entryid'])
op.create_table(
create_index_if_missing('idx_machinenumber_entry', 'manifestentrymachinenumbers',
['entryid'])
create_table_if_missing(
'manifestinusechecks',
sa.Column('inusecheckid', sa.Integer(), nullable=False),
sa.Column('entryid', sa.Integer(), nullable=False),
@@ -127,7 +132,7 @@ def upgrade():
sa.PrimaryKeyConstraint('inusecheckid'),
sa.UniqueConstraint('entryid'),
)
op.create_table(
create_table_if_missing(
'manifestinusecheckprocesses',
sa.Column('inusecheckprocessid', sa.Integer(), nullable=False),
sa.Column('inusecheckid', sa.Integer(), nullable=False),
@@ -140,9 +145,9 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('inusecheckprocessid'),
)
op.create_index('idx_process_check', 'manifestinusecheckprocesses',
['inusecheckid'])
op.create_table(
create_index_if_missing('idx_process_check', 'manifestinusecheckprocesses',
['inusecheckid'])
create_table_if_missing(
'manifestpublishedversions',
sa.Column('publishedversionid', sa.Integer(), nullable=False),
sa.Column('scopeid', sa.Integer(), nullable=False),
@@ -158,9 +163,9 @@ def upgrade():
sa.UniqueConstraint('scopeid', 'versionnumber',
name='uq_published_scope_version'),
)
op.create_index('idx_published_scope', 'manifestpublishedversions',
['scopeid'])
op.create_table(
create_index_if_missing('idx_published_scope', 'manifestpublishedversions',
['scopeid'])
create_table_if_missing(
'manifestpayloads',
sa.Column('payloadid', sa.Integer(), nullable=False),
sa.Column('entryid', sa.Integer(), nullable=False),
@@ -173,8 +178,8 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('payloadid'),
)
op.create_index('idx_payload_entry', 'manifestpayloads', ['entryid'])
op.create_table(
create_index_if_missing('idx_payload_entry', 'manifestpayloads', ['entryid'])
create_table_if_missing(
'manifestenforcementreports',
sa.Column('reportid', sa.Integer(), nullable=False),
sa.Column('hostname', sa.String(length=100), nullable=False),
@@ -192,13 +197,13 @@ def upgrade():
sa.Column('iscurrent', sa.Boolean(), nullable=False),
sa.PrimaryKeyConstraint('reportid'),
)
op.create_index('idx_report_host', 'manifestenforcementreports',
['hostname'])
op.create_index('idx_report_current', 'manifestenforcementreports',
['iscurrent'])
op.create_index('idx_report_host_scope', 'manifestenforcementreports',
['hostname', 'scopename', 'phase'])
op.create_table(
create_index_if_missing('idx_report_host', 'manifestenforcementreports',
['hostname'])
create_index_if_missing('idx_report_current', 'manifestenforcementreports',
['iscurrent'])
create_index_if_missing('idx_report_host_scope', 'manifestenforcementreports',
['hostname', 'scopename', 'phase'])
create_table_if_missing(
'manifestenforcementresults',
sa.Column('resultid', sa.Integer(), nullable=False),
sa.Column('reportid', sa.Integer(), nullable=False),
@@ -212,9 +217,9 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('resultid'),
)
op.create_index('idx_result_report', 'manifestenforcementresults',
['reportid'])
op.create_table(
create_index_if_missing('idx_result_report', 'manifestenforcementresults',
['reportid'])
create_table_if_missing(
'pctypealiases',
sa.Column('aliasid', sa.Integer(), nullable=False),
sa.Column('aliasgroup', sa.Integer(), nullable=False),
@@ -223,7 +228,7 @@ def upgrade():
sa.UniqueConstraint('aliasgroup', 'aliasname',
name='uq_alias_group_name'),
)
op.create_index('idx_alias_group', 'pctypealiases', ['aliasgroup'])
create_index_if_missing('idx_alias_group', 'pctypealiases', ['aliasgroup'])
def downgrade():

View File

@@ -10,6 +10,11 @@ Create Date: 2026-07-21
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -24,7 +29,7 @@ def upgrade():
insp = sa.inspect(bind)
if 'manifestblobs' in insp.get_table_names():
return
op.create_table(
create_table_if_missing(
'manifestblobs',
sa.Column('sha256', sa.String(length=64), nullable=False),
sa.Column('filename', sa.String(length=255), nullable=False),

View File

@@ -17,6 +17,11 @@ connection's default charset, so on a utf8mb4 database they are utf8mb4, matchin
how the core chain creates its tables.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -28,7 +33,7 @@ depends_on = None
def upgrade():
op.create_table(
create_table_if_missing(
'measuringtooltypes',
sa.Column('measuringtooltypeid', sa.Integer(), nullable=False),
sa.Column('name', sa.String(length=100), nullable=False),
@@ -40,7 +45,7 @@ def upgrade():
sa.PrimaryKeyConstraint('measuringtooltypeid'),
sa.UniqueConstraint('name'),
)
op.create_table(
create_table_if_missing(
'measuringtools',
sa.Column('measuringtoolid', sa.Integer(), nullable=False),
sa.Column('assetid', sa.Integer(), nullable=False),
@@ -59,8 +64,8 @@ def upgrade():
sa.PrimaryKeyConstraint('measuringtoolid'),
sa.UniqueConstraint('assetid'),
)
op.create_index('idx_measuringtool_type', 'measuringtools',
['measuringtooltypeid'])
create_index_if_missing('idx_measuringtool_type', 'measuringtools',
['measuringtooltypeid'])
def downgrade():

View File

@@ -11,6 +11,11 @@ ops are written out explicitly anyway to match the measuringtools exemplar
and keep the baseline reviewable.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -22,7 +27,7 @@ depends_on = None
def upgrade():
op.create_table(
create_table_if_missing(
'printeditems',
sa.Column('printeditemid', sa.Integer(), nullable=False),
sa.Column('itemcode', sa.String(length=20), nullable=True),
@@ -39,9 +44,9 @@ def upgrade():
sa.PrimaryKeyConstraint('printeditemid'),
sa.UniqueConstraint('itemcode'),
)
op.create_index('ix_printeditems_itemcode', 'printeditems', ['itemcode'])
create_index_if_missing('ix_printeditems_itemcode', 'printeditems', ['itemcode'])
op.create_table(
create_table_if_missing(
'printeditemtransactions',
sa.Column('transactionid', sa.Integer(), nullable=False),
sa.Column('printeditemid', sa.Integer(), nullable=False),
@@ -58,12 +63,12 @@ def upgrade():
ondelete='CASCADE'),
sa.PrimaryKeyConstraint('transactionid'),
)
op.create_index('ix_printeditemtransactions_printeditemid',
'printeditemtransactions', ['printeditemid'])
op.create_index('ix_printeditemtransactions_employeesso',
'printeditemtransactions', ['employeesso'])
op.create_index('ix_printeditemtransactions_transactiondate',
'printeditemtransactions', ['transactiondate'])
create_index_if_missing('ix_printeditemtransactions_printeditemid',
'printeditemtransactions', ['printeditemid'])
create_index_if_missing('ix_printeditemtransactions_employeesso',
'printeditemtransactions', ['employeesso'])
create_index_if_missing('ix_printeditemtransactions_transactiondate',
'printeditemtransactions', ['transactiondate'])
def downgrade():

View File

@@ -5,6 +5,11 @@ the ADR-008 payoff: the plugin evolves its own schema without touching
the core chain. Applied by `flask plugin upgrade-all`.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -16,7 +21,7 @@ depends_on = None
def upgrade():
op.create_table(
create_table_if_missing(
'printeditemfiles',
sa.Column('fileid', sa.Integer(), nullable=False),
sa.Column('printeditemid', sa.Integer(), nullable=False),
@@ -35,8 +40,8 @@ def upgrade():
sa.PrimaryKeyConstraint('fileid'),
sa.UniqueConstraint('storedfilename'),
)
op.create_index('ix_printeditemfiles_printeditemid',
'printeditemfiles', ['printeditemid'])
create_index_if_missing('ix_printeditemfiles_printeditemid',
'printeditemfiles', ['printeditemid'])
def downgrade():

View File

@@ -4,6 +4,11 @@ Stores the last tier alerted per (printer, supply) so the poller fires once
on a downward crossing and re-arms after a refill. Idempotent create.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -19,7 +24,7 @@ def upgrade():
inspector = sa.inspect(bind)
if 'printersupplyalerts' in inspector.get_table_names():
return
op.create_table(
create_table_if_missing(
'printersupplyalerts',
sa.Column('printersupplyalertid', sa.Integer(), primary_key=True),
sa.Column('printerid', sa.Integer(), nullable=False),
@@ -35,8 +40,8 @@ def upgrade():
sa.UniqueConstraint('printerid', 'supplykey',
name='uq_printersupplyalert_printer_key'),
)
op.create_index('ix_printersupplyalerts_printerid',
'printersupplyalerts', ['printerid'])
create_index_if_missing('ix_printersupplyalerts_printerid',
'printersupplyalerts', ['printerid'])
def downgrade():

View File

@@ -21,6 +21,11 @@ Revision ID: printers0005observedqueues
Revises: printers0004drivervendor
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -36,7 +41,7 @@ def upgrade():
inspector = sa.inspect(bind)
if 'printerobservedqueues' in inspector.get_table_names():
return
op.create_table(
create_table_if_missing(
'printerobservedqueues',
sa.Column('printerobservedqueueid', sa.Integer(), nullable=False),
# Nullable and no index of its own beyond the explicit one below: an
@@ -67,10 +72,10 @@ def upgrade():
)
# Port address is the primary match key from an observed queue back to a
# printer asset, so every comparison read hits it.
op.create_index('idx_printerobservedqueues_portaddress',
'printerobservedqueues', ['portaddress'])
op.create_index('idx_printerobservedqueues_assetid',
'printerobservedqueues', ['assetid'])
create_index_if_missing('idx_printerobservedqueues_portaddress',
'printerobservedqueues', ['portaddress'])
create_index_if_missing('idx_printerobservedqueues_assetid',
'printerobservedqueues', ['assetid'])
def downgrade():

View File

@@ -12,6 +12,11 @@ plugin's tables, so the foreign key to the core users table cannot resolve at
CreateTable-compile time.
"""
from alembic import op
from shopdb.plugins.alembic_template import (
create_index_if_missing,
create_table_if_missing,
)
import sqlalchemy as sa
@@ -23,7 +28,7 @@ depends_on = None
def upgrade():
op.create_table(
create_table_if_missing(
'securesendsecrets',
sa.Column('secretid', sa.Integer(), nullable=False),
# 128 bits of randomness as hex. With no login on the read side this is
@@ -47,13 +52,13 @@ def upgrade():
)
# Unique, because the token is looked up on every read and two rows sharing
# one would make a link ambiguous.
op.create_index('ixsecuresendsecretstoken', 'securesendsecrets',
['token'], unique=True)
create_index_if_missing('ixsecuresendsecretstoken', 'securesendsecrets',
['token'], unique=True)
# Expiry sweeps scan on this.
op.create_index('ixsecuresendsecretsexpiresat', 'securesendsecrets',
['expiresat'])
op.create_index('ixsecuresendsecretscreatedby', 'securesendsecrets',
['createdbyuserid'])
create_index_if_missing('ixsecuresendsecretsexpiresat', 'securesendsecrets',
['expiresat'])
create_index_if_missing('ixsecuresendsecretscreatedby', 'securesendsecrets',
['createdbyuserid'])
def downgrade():