Plugin baselines skip tables that already exist
This commit is contained in:
14
CHANGELOG.md
14
CHANGELOG.md
@@ -105,6 +105,20 @@ ADR-007 and ADR-002.
|
||||
the wrong code. The page index now travels with the page.
|
||||
- The back label's dotted preview outline outspecified the print reset and drew
|
||||
a dashed rule down the edge of every second card.
|
||||
- **`flask plugin upgrade-all` failed a plugin whose tables already existed**
|
||||
("table securesendsecrets already exists" on the tools plugin). Every
|
||||
post-cutover baseline emitted a plain `op.create_table`, which is correct only
|
||||
while a table's existence and its plugin's Alembic stamp cannot disagree.
|
||||
They can: anything that builds tables from the models stamps nothing, and a
|
||||
run that creates a table then fails before its stamp commits leaves the same
|
||||
split, where re-running is the obvious response. Creates and indexes across
|
||||
all nine table-creating plugin migrations now skip what is already there.
|
||||
Indexes are matched on their COLUMNS rather than their name, because
|
||||
`db.create_all()` names them by SQLAlchemy's convention and a migration by
|
||||
this repo's, so a name-keyed check would silently add a duplicate index
|
||||
instead of skipping. Note the remaining limit: this makes a baseline
|
||||
idempotent, not a whole chain. A longer chain can still meet an `ALTER` whose
|
||||
column is already present, which no guard on `create_table` addresses.
|
||||
- **Saving a machine that has a controlling PC answered "this relationship
|
||||
already exists" and refused the save.** Reported against a map position
|
||||
change, but the map was incidental: the form re-POSTed the controlling-PC
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -104,7 +104,12 @@ def create_plugin_tables(plugin_name: str):
|
||||
this is safe on an existing database that has the table from the pre-cutover
|
||||
core baseline as well as on a fresh install (ADR-014 Phase 2).
|
||||
|
||||
Called from each plugin's 0001 baseline.py upgrade().
|
||||
NOT actually called from anywhere: every post-cutover baseline emits
|
||||
explicit Alembic ops instead, because this helper's per-plugin MetaData
|
||||
cannot resolve a foreign key to a CORE table at CreateTable-compile time.
|
||||
Kept because it is still the reference for what idempotent means here, and
|
||||
because create_table_if_missing below is the same rule in a form the
|
||||
baselines can use. See that function for what went wrong without it.
|
||||
"""
|
||||
from alembic import op
|
||||
from sqlalchemy import inspect
|
||||
@@ -120,6 +125,67 @@ def create_plugin_tables(plugin_name: str):
|
||||
op.execute(str(CreateTable(table).compile(dialect=bind.dialect)))
|
||||
|
||||
|
||||
def create_table_if_missing(table_name: str, *columns, **kwargs) -> bool:
|
||||
"""op.create_table, skipped when the table is already there.
|
||||
|
||||
WHY EVERY POST-CUTOVER BASELINE NEEDS THIS. A baseline is the sole creator
|
||||
of its plugin's tables, so it was written as a plain op.create_table - which
|
||||
is correct only if the table's existence and the plugin's Alembic stamp can
|
||||
never disagree. They can, and routinely do:
|
||||
|
||||
* `flask db-utils create-all` runs db.create_all(), which builds EVERY
|
||||
table registered on the metadata - plugin tables included - and stamps
|
||||
NOTHING. `flask plugin upgrade-all` then runs the baseline against
|
||||
tables that already exist.
|
||||
* A run that creates the table and then fails before its stamp commits
|
||||
leaves the same split, and re-running is the obvious thing to try.
|
||||
|
||||
Reported as `flask plugin upgrade-all` failing the tools plugin with
|
||||
"table securesendsecrets already exists". Nothing caught it because the
|
||||
migration test drops the post-cutover tables db.create_all() made before
|
||||
running upgrade-all, which arranges the one state where an unguarded create
|
||||
works and never the state a real database is in.
|
||||
|
||||
Returns True when it created the table, so a caller can skip the indexes
|
||||
that belong to it.
|
||||
"""
|
||||
from alembic import op
|
||||
from sqlalchemy import inspect
|
||||
|
||||
if inspect(op.get_bind()).has_table(table_name):
|
||||
logger.info('%s already exists, skipping create', table_name)
|
||||
return False
|
||||
op.create_table(table_name, *columns, **kwargs)
|
||||
return True
|
||||
|
||||
|
||||
def create_index_if_missing(index_name: str, table_name: str, columns, **kwargs):
|
||||
"""op.create_index, skipped when an equivalent index already exists.
|
||||
|
||||
Matched on the COLUMN LIST rather than the name, because the two creators
|
||||
do not agree on names. db.create_all() takes them from SQLAlchemy's default
|
||||
convention (`ix_securesendsecrets_token`) while a baseline spells them the
|
||||
way this repo's naming rules require (`ixsecuresendsecretstoken`). Matching
|
||||
on the name would call the index missing and add a second one over the same
|
||||
column - MySQL permits that, with only a warning, so the duplicate would
|
||||
accumulate silently on every affected site.
|
||||
"""
|
||||
from alembic import op
|
||||
from sqlalchemy import inspect
|
||||
|
||||
wanted = [str(column) for column in columns]
|
||||
inspector = inspect(op.get_bind())
|
||||
if not inspector.has_table(table_name):
|
||||
op.create_index(index_name, table_name, columns, **kwargs)
|
||||
return
|
||||
for existing in inspector.get_indexes(table_name):
|
||||
if list(existing.get('column_names') or []) == wanted:
|
||||
logger.info('index over %s(%s) already exists as %s, skipping',
|
||||
table_name, ', '.join(wanted), existing.get('name'))
|
||||
return
|
||||
op.create_index(index_name, table_name, columns, **kwargs)
|
||||
|
||||
|
||||
def drop_plugin_tables(plugin_name: str):
|
||||
"""Mirror of create_plugin_tables for downgrade(). Drops in reverse FK
|
||||
order."""
|
||||
|
||||
@@ -276,3 +276,89 @@ def test_upgrade_all_on_fresh_db_is_clean_and_idempotent(tmp_path, monkeypatch):
|
||||
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
||||
plugin_manager.loader, plugin_manager.migration_manager,
|
||||
plugin_manager._registered_prefixes) = saved
|
||||
|
||||
|
||||
|
||||
# Plugins whose whole chain is a single baseline. Their tables existing already
|
||||
# is the ONLY divergence possible between schema and stamp, so upgrade-all must
|
||||
# handle it end to end. A longer chain can also meet an ALTER whose column is
|
||||
# already present, which no amount of guarding a create_table fixes - see the
|
||||
# test below for why that is a different problem.
|
||||
SINGLE_BASELINE_PLUGINS = tuple(
|
||||
plugin for plugin in POST_CUTOVER_PLUGINS
|
||||
if len(list((PLUGINS_DIR / plugin / 'migrations' / 'versions').glob('0*.py'))) == 1
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('plugin', SINGLE_BASELINE_PLUGINS)
|
||||
def test_baseline_skips_tables_that_already_exist(plugin, tmp_path, monkeypatch):
|
||||
"""A baseline must stamp, not fail, when its table is already there.
|
||||
|
||||
Reported: `flask plugin upgrade-all` failed the tools plugin with "table
|
||||
securesendsecrets already exists". Every post-cutover baseline was a plain
|
||||
op.create_table, correct only while a table's existence and its plugin's
|
||||
Alembic stamp can never disagree. They can - anything that builds tables
|
||||
from the models (db.create_all) stamps nothing, and a run that creates a
|
||||
table then fails before its stamp commits leaves the same split.
|
||||
|
||||
The other upgrade-all test DROPS these tables before running, so it only
|
||||
ever covered the state where they are absent. This is the other state.
|
||||
"""
|
||||
from sqlalchemy import inspect, text
|
||||
from shopdb.config import TestingConfig
|
||||
from shopdb import create_app
|
||||
from shopdb.extensions import db
|
||||
from shopdb.plugins import plugin_manager
|
||||
|
||||
monkeypatch.setattr(TestingConfig, 'SQLALCHEMY_DATABASE_URI',
|
||||
f'sqlite:///{tmp_path / "preexisting.db"}')
|
||||
|
||||
saved = (plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
||||
plugin_manager.loader, plugin_manager.migration_manager,
|
||||
plugin_manager._registered_prefixes)
|
||||
try:
|
||||
app = create_app('testing')
|
||||
with app.app_context():
|
||||
db.create_all()
|
||||
|
||||
# Leave THIS plugin's tables in place - that is the whole point -
|
||||
# and drop the other post-cutover ones so their own (longer) chains
|
||||
# run the way the sibling test already covers.
|
||||
insp0 = inspect(db.engine)
|
||||
for other in POST_CUTOVER_PLUGINS:
|
||||
if other == plugin:
|
||||
continue
|
||||
for tablename in PLUGIN_TABLE_OWNERS[other]:
|
||||
if insp0.has_table(tablename):
|
||||
db.session.execute(text(f'DROP TABLE {tablename}'))
|
||||
db.session.commit()
|
||||
|
||||
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
|
||||
assert inspect(db.engine).has_table(tablename), (
|
||||
f'{tablename} should have been built by create_all; '
|
||||
f'this test is meaningless without it')
|
||||
|
||||
first = app.extensions['plugin_manager'].upgrade_all_plugins()
|
||||
assert first[plugin] == 'ok', first[plugin]
|
||||
|
||||
second = app.extensions['plugin_manager'].upgrade_all_plugins()
|
||||
assert second[plugin] == 'ok', second[plugin]
|
||||
|
||||
insp = inspect(db.engine)
|
||||
row = db.session.execute(
|
||||
text(f'SELECT version_num FROM alembic_version_{plugin}')
|
||||
).fetchone()
|
||||
assert row and row[0] == EXPECTED_HEAD_REVISION[plugin]
|
||||
|
||||
# No second index over the same columns: create_all names indexes by
|
||||
# SQLAlchemy's convention and the baseline by this repo's, so a
|
||||
# name-keyed guard would have added a duplicate rather than skipped.
|
||||
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
|
||||
seen = [tuple(ix.get('column_names') or ())
|
||||
for ix in insp.get_indexes(tablename)]
|
||||
assert len(seen) == len(set(seen)), (
|
||||
f'{tablename} has two indexes over the same columns: {seen}')
|
||||
finally:
|
||||
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
||||
plugin_manager.loader, plugin_manager.migration_manager,
|
||||
plugin_manager._registered_prefixes) = saved
|
||||
|
||||
Reference in New Issue
Block a user