From 2c7c0cab06adcfd5e510985dc1af302a47ae9899 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 16 Jul 2021 13:04:01 +0100 Subject: tests: Enable SAWarning warnings We shouldn't be raising warnings from SQLAlchemy. Where we are intentionally doing so, we should capture these warnings at the test level. This requires some minor fixes. Change-Id: I9d4512dc337153edc48a2cc3bf95ab2b31c39ccf Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/utils.py | 18 +++++++++++++++--- oslo_db/tests/fixtures.py | 5 +++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index e5b3531..f1eec42 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -39,6 +39,7 @@ from sqlalchemy import Index from sqlalchemy import inspect from sqlalchemy import Integer from sqlalchemy import MetaData +from sqlalchemy import PrimaryKeyConstraint from sqlalchemy.sql.expression import cast from sqlalchemy.sql.expression import literal_column from sqlalchemy.sql import text @@ -608,7 +609,14 @@ def _change_deleted_column_type_to_boolean_sqlite(engine, table_name, # FIXME(stephenfin): We shouldn't be using this private API; # figure out how else to copy an arbitrary column schema - constraints = [constraint._copy() for constraint in table.constraints] + # NOTE(stephenfin): We drop PrimaryKeyConstraint-type constraints since + # these duplicate the 'primary_key=True' attribute on the speicified + # column(s). This technically breaks things when the primary key covers + # multiple columns but that's okay: these are deprecated APIs + constraints = [ + constraint._copy() for constraint in table.constraints + if not isinstance(constraint, PrimaryKeyConstraint) + ] with engine.connect() as conn: meta = table.metadata @@ -738,7 +746,10 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, constraints = [] for constraint in table.constraints: - if not _is_deleted_column_constraint(constraint): + if not ( + _is_deleted_column_constraint(constraint) or + isinstance(constraint, PrimaryKeyConstraint) + ): # FIXME(stephenfin): We shouldn't be using this private API; # figure out how else to copy an arbitrary constraint schema constraints.append(constraint._copy()) @@ -749,7 +760,8 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, with conn.begin(): new_table = Table( table_name + "__tmp__", meta, - *(columns + constraints)) + *(columns + constraints), + ) new_table.create(conn) indexes = [] diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 93cdcb6..4cc596f 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -24,11 +24,12 @@ class WarningsFixture(fixtures.Fixture): self._original_warning_filters = warnings.filters[:] - # Make deprecation warnings only happen once to avoid spamming warnings.simplefilter('once', DeprecationWarning) + # Enable generic warnings to ensure we're not doing anything odd + warnings.filterwarnings( - 'error', message='Evaluating non-mapped column expression', + 'error', category=sqla_exc.SAWarning) # Enable deprecation warnings to capture upcoming SQLAlchemy changes -- cgit v1.2.1