From 0a5b2d66ca08abbea66f8498a71e0bdb78b2b30a Mon Sep 17 00:00:00 2001 From: Kamil Páral Date: Jul 29 2021 12:04:54 +0000 Subject: SQLAlchemy: add naming conventions Without naming conventions, column constraints created with e.g. `unique=True` are not named, and therefore future edits of those constraints (or e.g. database downgrades) fail, unless you figure out the constraint name used in your particular database and add it into each new Alembic script. A better approach is to define a naming convention. SQLAlchemy then names all constraints according to this convention, and Alembic scripts work out-of-the-box even for constraint changes and downgrades. You can read more about it at: https://docs.sqlalchemy.org/en/14/core/constraints.html#configuring-constraint-naming-conventions This patch adds a default naming convention (as suggested by SQLAlchemy docs), and adds an Alembic script that performs a one-time rename of all affected constraints. Unique constraints were detected by Alembic's autogeneration feature, primary and foreign keys were renamed manually. Merges: https://pagure.io/fedora-qa/blockerbugs/pull-request/194 --- diff --git a/alembic/versions/bc58ec7c7d31_add_db_naming_conventions.py b/alembic/versions/bc58ec7c7d31_add_db_naming_conventions.py new file mode 100644 index 0000000..d9cc15b --- /dev/null +++ b/alembic/versions/bc58ec7c7d31_add_db_naming_conventions.py @@ -0,0 +1,102 @@ +"""Add DB naming conventions + +Revision ID: bc58ec7c7d31 +Revises: d7b6117a119a +Create Date: 2021-07-22 18:21:27.437519 + +""" +from alembic import op + +# revision identifiers, used by Alembic. +revision = 'bc58ec7c7d31' +down_revision = 'd7b6117a119a' + +# constraint renames {table_name: [(from, to)]} +renames = { + 'bug': [ + ('bug_pkey', 'pk_bug'), + ('bug_milestone_id_fkey', 'fk_bug_milestone_id_milestone') + ], + 'build': [ + ('build_pkey', 'pk_build'), + ], + 'criterion': [ + ('criterion_pkey', 'pk_criterion'), + ('criterion_milestone_id_fkey', 'fk_criterion_milestone_id_milestone'), + ], + 'milestone': [ + ('milestone_pkey', 'pk_milestone'), + ('milestone_release_id_fkey', 'fk_milestone_release_id_release'), + ('milestone_succeeds_id_fkey', 'fk_milestone_succeeds_id_milestone'), + ], + 'release': [ + ('release_pkey', 'pk_release'), + ], + 'update': [ + ('update_pkey', 'pk_update'), + ('update_release_id_fkey', 'fk_update_release_id_release'), + ], + 'update_fixes': [ + ('update_fixes_bug_id_fkey', 'fk_update_fixes_bug_id_bug'), + ('update_fixes_update_id_fkey', 'fk_update_fixes_update_id_update'), + ], + 'update_milestones': [ + ('update_milestones_milestone_id_fkey', 'fk_update_milestones_milestone_id_milestone'), + ('update_milestones_update_id_fkey', 'fk_update_milestones_update_id_update'), + ], + 'user_info': [ + ('user_info_pkey', 'pk_user_info'), + ], +} + + +def upgrade(): + # rename all constraints which are not autodetected by Alembic + for table, renamelist in renames.items(): + for from_, to in renamelist: + op.execute(f'ALTER TABLE "{table}" RENAME CONSTRAINT {from_} TO {to}') + + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('build_nvr_key', 'build', type_='unique') + op.drop_constraint('build_pkg_ver_rel', 'build', type_='unique') + op.create_unique_constraint(op.f('uq_build_nvr'), 'build', ['nvr']) + op.create_unique_constraint('uq_build_pkg_ver_rel', 'build', ['pkg_name', 'version', 'release']) + op.drop_constraint('milestone_blocker_tracker_key', 'milestone', type_='unique') + op.drop_constraint('milestone_fe_tracker_key', 'milestone', type_='unique') + op.drop_constraint('milestone_name_key', 'milestone', type_='unique') + op.create_unique_constraint(op.f('uq_milestone_blocker_tracker'), 'milestone', ['blocker_tracker']) + op.create_unique_constraint(op.f('uq_milestone_fe_tracker'), 'milestone', ['fe_tracker']) + op.create_unique_constraint(op.f('uq_milestone_name'), 'milestone', ['name']) + op.drop_constraint('user_info_api_key_hash_key', 'user_info', type_='unique') + op.drop_constraint('user_info_bz_user_key', 'user_info', type_='unique') + op.drop_constraint('user_info_fas_login_key', 'user_info', type_='unique') + op.create_unique_constraint(op.f('uq_user_info_api_key_hash'), 'user_info', ['api_key_hash']) + op.create_unique_constraint(op.f('uq_user_info_bz_user'), 'user_info', ['bz_user']) + op.create_unique_constraint(op.f('uq_user_info_fas_login'), 'user_info', ['fas_login']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(op.f('uq_user_info_fas_login'), 'user_info', type_='unique') + op.drop_constraint(op.f('uq_user_info_bz_user'), 'user_info', type_='unique') + op.drop_constraint(op.f('uq_user_info_api_key_hash'), 'user_info', type_='unique') + op.create_unique_constraint('user_info_fas_login_key', 'user_info', ['fas_login']) + op.create_unique_constraint('user_info_bz_user_key', 'user_info', ['bz_user']) + op.create_unique_constraint('user_info_api_key_hash_key', 'user_info', ['api_key_hash']) + op.drop_constraint(op.f('uq_milestone_name'), 'milestone', type_='unique') + op.drop_constraint(op.f('uq_milestone_fe_tracker'), 'milestone', type_='unique') + op.drop_constraint(op.f('uq_milestone_blocker_tracker'), 'milestone', type_='unique') + op.create_unique_constraint('milestone_name_key', 'milestone', ['name']) + op.create_unique_constraint('milestone_fe_tracker_key', 'milestone', ['fe_tracker']) + op.create_unique_constraint('milestone_blocker_tracker_key', 'milestone', ['blocker_tracker']) + op.drop_constraint('uq_build_pkg_ver_rel', 'build', type_='unique') + op.drop_constraint(op.f('uq_build_nvr'), 'build', type_='unique') + op.create_unique_constraint('build_pkg_ver_rel', 'build', ['pkg_name', 'version', 'release']) + op.create_unique_constraint('build_nvr_key', 'build', ['nvr']) + # ### end Alembic commands ### + + # rename all constraints which are not autodetected by Alembic + for table, renamelist in renames.items(): + for from_, to in renamelist: + op.execute(f'ALTER TABLE "{table}" RENAME CONSTRAINT {to} TO {from_}') diff --git a/blockerbugs/__init__.py b/blockerbugs/__init__.py index 3b8b810..8a7df65 100644 --- a/blockerbugs/__init__.py +++ b/blockerbugs/__init__.py @@ -6,6 +6,7 @@ import os from flask import Flask, render_template from flask_sqlalchemy import SQLAlchemy from flask_sqlalchemy.model import DefaultMeta +import sqlalchemy from . import config @@ -105,7 +106,16 @@ setup_logging() # database if app.config['SHOW_DB_URI']: app.logger.debug('using DBURI: %s' % app.config['SQLALCHEMY_DATABASE_URI']) -db: SQLAlchemy = SQLAlchemy(app) +metadata = sqlalchemy.MetaData( + naming_convention={ + 'pk': 'pk_%(table_name)s', + 'fk': 'fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s', + 'ix': 'ix_%(column_0_label)s', + 'uq': 'uq_%(table_name)s_%(column_0_name)s', + 'ck': 'ck_%(table_name)s_%(constraint_name)s', + } +) +db: SQLAlchemy = SQLAlchemy(app=app, metadata=metadata) # https://stackoverflow.com/questions/63542818/mypy-and-inheriting-from-a-class-that-is-an-attribute-on-an-instance BaseModel: DefaultMeta = db.Model diff --git a/blockerbugs/models/build.py b/blockerbugs/models/build.py index 9b8a44c..52c49fe 100644 --- a/blockerbugs/models/build.py +++ b/blockerbugs/models/build.py @@ -36,7 +36,7 @@ class Build(BaseModel): epoch = db.Column(db.Integer, nullable=True) __table_args__ = (UniqueConstraint('pkg_name', 'version', 'release', - name='build_pkg_ver_rel'), ) + name='uq_build_pkg_ver_rel'), ) def __unicode__(self): return 'build: %s' % (self.nvr)