From 5544f1033fdbbfe7fba170c82407df0dbb78ad10 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 16 Jul 2021 19:25:34 +0100 Subject: [PATCH 01/16] Don't use dict-style attribute accesses Resolve the following RemovedIn20Warning warning: Using non-integer/slice indices on Row is deprecated and will be removed in version 2.0; please use row._mapping[], or the mappings() accessor on the Result object. Change-Id: I3a4845216914635e5802a70c2b1be757d82b7a49 Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/utils.py | 2 +- oslo_db/tests/fixtures.py | 5 ----- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 2 +- oslo_db/tests/sqlalchemy/test_utils.py | 12 +++++++----- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 99da62b..2ab7188 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -496,7 +496,7 @@ def drop_old_duplicate_entries_from_table(engine, table_name, is_none = None # workaround for pyflakes delete_condition &= table.c.deleted_at == is_none for name in uc_column_names: - delete_condition &= table.c[name] == row[name] + delete_condition &= table.c[name] == row._mapping[name] rows_to_delete_select = sqlalchemy.sql.select( table.c.id, diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index a1b6929..7db7b78 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -47,11 +47,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Session.begin.subtransactions flag is deprecated .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'Using non-integer/slice indices on Row is deprecated .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'The Engine.execute\(\) method is considered legacy .*', diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index 10def6b..ede67c6 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -327,7 +327,7 @@ class MySQLModeTestCase(db_test_base._MySQLOpportunisticTestCase): self.connection.execute(self.test_table.insert(), bar=value) result = self.connection.execute(self.test_table.select()) - return result.fetchone()['bar'] + return result.fetchone().bar def test_string_too_long(self): value = 'a' * 512 diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 7531332..2f61004 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -761,17 +761,19 @@ class TestMigrationUtils(db_test_base._DbTestCase): base_select = table.select() rows_select = base_select.where(table.c.deleted != table.c.id) - row_ids = [row['id'] for row in - self.engine.execute(rows_select).fetchall()] + row_ids = [ + row.id for row in self.engine.execute(rows_select).fetchall() + ] self.assertEqual(len(expected_values), len(row_ids)) for value in expected_values: self.assertIn(value['id'], row_ids) deleted_rows_select = base_select.where( table.c.deleted == table.c.id) - deleted_rows_ids = [row['id'] for row in - self.engine.execute( - deleted_rows_select).fetchall()] + deleted_rows_ids = [ + row.id for row in + self.engine.execute(deleted_rows_select).fetchall() + ] self.assertEqual(len(values) - len(row_ids), len(deleted_rows_ids)) for value in soft_deleted_values: -- GitLab From 8772fcc349226b152e8a04a8ed3e883231412043 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 10:37:39 +0100 Subject: [PATCH 02/16] Don't use the 'Row.keys()' method Resolve the following RemovedIn20Warning warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use row._mapping.keys() Change-Id: I647a57909df56fec7b570ae29efbc731126df14d Signed-off-by: Stephen Finucane --- oslo_db/tests/fixtures.py | 5 ----- oslo_db/tests/sqlalchemy/test_update_match.py | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 7db7b78..20b8a38 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -57,11 +57,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Executable.execute\(\) method is considered .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The Row.keys\(\) method is considered legacy .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'Retrieving row members using strings or other .*', diff --git a/oslo_db/tests/sqlalchemy/test_update_match.py b/oslo_db/tests/sqlalchemy/test_update_match.py index fdd1887..b4c025b 100644 --- a/oslo_db/tests/sqlalchemy/test_update_match.py +++ b/oslo_db/tests/sqlalchemy/test_update_match.py @@ -122,7 +122,7 @@ class UpdateMatchTest(db_test_base._DbTestCase): sql.select(MyModel.__table__).where(MyModel.__table__.c.id == pk) ).first() values['id'] = pk - self.assertEqual(values, dict(row)) + self.assertEqual(values, dict(row._mapping)) def test_update_specimen_successful(self): uuid = '136254d5-3869-408f-9da7-190e0072641a' -- GitLab From fadc0ef9022a360016d2e9af756fb93c9c308467 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 10:40:32 +0100 Subject: [PATCH 03/16] Replace use of Engine.scalar() Resolve the following RemovedIn20Warning warning: The Engine.scalar() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. All statement execution in SQLAlchemy 2.0 is performed by the Connection.execute() method of Connection, or in the ORM by the Session.execute() method of Session; the Result.scalar() method can then be used to return a scalar result. Change-Id: Ic3c9c5bb008b6299f2ed9a59bda0329fab9d554d Signed-off-by: Stephen Finucane --- oslo_db/tests/fixtures.py | 5 ----- oslo_db/tests/sqlalchemy/test_exc_filters.py | 11 ++++++++--- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 20b8a38..e1a41c9 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -82,11 +82,6 @@ class WarningsFixture(fixtures.Fixture): message=r'Calling \.begin\(\) when a transaction is already .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The Engine.scalar\(\) method is considered legacy .*', - category=sqla_exc.SADeprecationWarning) - # ...plus things that aren't our fault # FIXME(stephenfin): These are caused by sqlalchemy-migrate, not us, diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index 9075f2a..a05c9a9 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -1162,11 +1162,14 @@ class TestDBDisconnected(TestsExceptionFilter): yield def _test_ping_listener_disconnected( - self, dialect_name, exc_obj, is_disconnect=True): + self, dialect_name, exc_obj, is_disconnect=True, + ): with self._fixture(dialect_name, exc_obj, 1, is_disconnect): conn = self.engine.connect() with conn.begin(): - self.assertEqual(1, conn.scalar(sqla.select(1))) + self.assertEqual( + 1, conn.execute(sqla.select(1)).scalars().first(), + ) self.assertFalse(conn.closed) self.assertFalse(conn.invalidated) self.assertTrue(conn.in_transaction()) @@ -1179,7 +1182,9 @@ class TestDBDisconnected(TestsExceptionFilter): # test implicit execution with self._fixture(dialect_name, exc_obj, 1): - self.assertEqual(1, self.engine.scalar(sqla.select(1))) + self.assertEqual( + 1, self.engine.execute(sqla.select(1)).scalars().first(), + ) def test_mariadb_error_1927(self): for code in [1927]: diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index ede67c6..003a49a 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -479,14 +479,14 @@ class SQLiteConnectTest(test_base.BaseTestCase): engine = self._fixture(sqlite_fk=True) self.assertEqual( 1, - engine.scalar(sql.text('pragma foreign_keys')) + engine.execute(sql.text('pragma foreign_keys')).scalars().first(), ) engine = self._fixture(sqlite_fk=False) self.assertEqual( 0, - engine.scalar(sql.text("pragma foreign_keys")) + engine.execute(sql.text('pragma foreign_keys')).scalars().first(), ) def test_sqlite_synchronous_listener(self): @@ -496,14 +496,14 @@ class SQLiteConnectTest(test_base.BaseTestCase): # http://www.sqlite.org/pragma.html#pragma_synchronous self.assertEqual( 2, - engine.scalar(sql.text('pragma synchronous')) + engine.execute(sql.text('pragma synchronous')).scalars().first(), ) engine = self._fixture(sqlite_synchronous=False) self.assertEqual( 0, - engine.scalar(sql.text('pragma synchronous')) + engine.execute(sql.text('pragma synchronous')).scalars().first(), ) -- GitLab From c320df474636ac22e9e778b1cce8975afae8100c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 10:52:27 +0100 Subject: [PATCH 04/16] Remove unnecessary warning filter Looks like we solved this along the way. Change-Id: I50469e58fe3fae695316cb344fea44ec18fffe60 Signed-off-by: Stephen Finucane --- oslo_db/tests/fixtures.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index e1a41c9..f14bc1c 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -57,11 +57,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Executable.execute\(\) method is considered .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'Retrieving row members using strings or other .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'The connection.execute\(\) method in SQLAlchemy 2.0 .*', -- GitLab From 62d77fc6df9bf27a334613d42a2ffa14c9711928 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 10:53:01 +0100 Subject: [PATCH 05/16] Replace use of Executable.execute method Resolve the following RemovedIn20Warning warning: The Executable.execute() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. All statement execution in SQLAlchemy 2.0 is performed by the Connection.execute() method of Connection, or in the ORM by the Session.execute() method of Session. Change-Id: Ie0acba4a315c85ec7236e44a22449e0ad920ca9b Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/utils.py | 158 +++++++++++++++++++----------------- oslo_db/tests/fixtures.py | 5 -- 2 files changed, 83 insertions(+), 80 deletions(-) diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 2ab7188..180b07f 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -569,11 +569,12 @@ def change_deleted_column_type_to_boolean(engine, table_name, finally: table.metadata.bind = None - engine.execute( - table.update(). - where(table.c.deleted == table.c.id). - values(old_deleted=True) - ) + with engine.connect() as conn: + conn.execute( + table.update().where( + table.c.deleted == table.c.id + ).values(old_deleted=True) + ) table.metadata.bind = engine try: @@ -607,39 +608,42 @@ def _change_deleted_column_type_to_boolean_sqlite(engine, table_name, # figure out how else to copy an arbitrary column schema constraints = [constraint._copy() for constraint in table.constraints] - meta = table.metadata - new_table = Table(table_name + "__tmp__", meta, - *(columns + constraints)) - new_table.create(engine) + with engine.connect() as conn: + meta = table.metadata + new_table = Table( + table_name + "__tmp__", meta, + *(columns + constraints)) + new_table.create(conn) - indexes = [] - for index in get_indexes(engine, table_name): - column_names = [new_table.c[c] for c in index['column_names']] - indexes.append(Index(index["name"], *column_names, - unique=index["unique"])) - - c_select = [] - for c in table.c: - if c.name != "deleted": - c_select.append(c) - else: - c_select.append(table.c.deleted == table.c.id) + indexes = [] + for index in get_indexes(engine, table_name): + column_names = [new_table.c[c] for c in index['column_names']] + indexes.append( + Index(index["name"], *column_names, unique=index["unique"]) + ) + + c_select = [] + for c in table.c: + if c.name != "deleted": + c_select.append(c) + else: + c_select.append(table.c.deleted == table.c.id) - table.drop(engine) - for index in indexes: - index.create(engine) + table.drop(conn) + for index in indexes: + index.create(conn) - table.metadata.bind = engine - try: - new_table.rename(table_name) - finally: - table.metadata.bind = None - - engine.execute( - new_table.update(). - where(new_table.c.deleted == new_table.c.id). - values(deleted=True) - ) + table.metadata.bind = engine + try: + new_table.rename(table_name) + finally: + table.metadata.bind = None + + conn.execute( + new_table.update().where( + new_table.c.deleted == new_table.c.id + ).values(deleted=True) + ) @debtcollector.removals.remove( @@ -664,21 +668,22 @@ def change_deleted_column_type_to_id_type(engine, table_name, finally: table.metadata.bind = None - deleted = True # workaround for pyflakes - engine.execute( - table.update(). - where(table.c.deleted == deleted). - values(new_deleted=table.c.id) - ) table.metadata.bind = engine try: - table.c.deleted.drop() - table.c.new_deleted.alter(name="deleted") + with engine.connect() as conn: + deleted = True # workaround for pyflakes + conn.execute( + table.update().where( + table.c.deleted == deleted + ).values(new_deleted=table.c.id) + ) + table.c.deleted.drop() + table.c.new_deleted.alter(name="deleted") + + _restore_indexes_on_deleted_columns(conn, table_name, indexes) finally: table.metadata.bind = None - _restore_indexes_on_deleted_columns(engine, table_name, indexes) - def _is_deleted_column_constraint(constraint): # NOTE(boris-42): There is no other way to check is CheckConstraint @@ -731,40 +736,43 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, # figure out how else to copy an arbitrary constraint schema constraints.append(constraint._copy()) - new_table = Table(table_name + "__tmp__", meta, - *(columns + constraints)) - new_table.create(engine) + with engine.connect() as conn: + new_table = Table( + table_name + "__tmp__", meta, + *(columns + constraints)) + new_table.create(conn) - indexes = [] - for index in get_indexes(engine, table_name): - column_names = [new_table.c[c] for c in index['column_names']] - indexes.append(Index(index["name"], *column_names, - unique=index["unique"])) - - table.drop(engine) - for index in indexes: - index.create(engine) + indexes = [] + for index in get_indexes(engine, table_name): + column_names = [new_table.c[c] for c in index['column_names']] + indexes.append( + Index(index["name"], *column_names, unique=index["unique"]) + ) - new_table.metadata.bind = engine - try: - new_table.rename(table_name) - finally: - new_table.metadata.bind = None + table.drop(conn) + for index in indexes: + index.create(conn) - deleted = True # workaround for pyflakes - engine.execute( - new_table.update(). - where(new_table.c.deleted == deleted). - values(deleted=new_table.c.id) - ) + new_table.metadata.bind = engine + try: + new_table.rename(table_name) + finally: + new_table.metadata.bind = None + + deleted = True # workaround for pyflakes + conn.execute( + new_table.update().where( + new_table.c.deleted == deleted + ).values(deleted=new_table.c.id) + ) - # NOTE(boris-42): Fix value of deleted column: False -> "" or 0. - deleted = False # workaround for pyflakes - engine.execute( - new_table.update(). - where(new_table.c.deleted == deleted). - values(deleted=default_deleted_value) - ) + # NOTE(boris-42): Fix value of deleted column: False -> "" or 0. + deleted = False # workaround for pyflakes + conn.execute( + new_table.update().where( + new_table.c.deleted == deleted + ).values(deleted=default_deleted_value) + ) def get_db_connection_info(conn_pieces): diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index f14bc1c..561ebc5 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -52,11 +52,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Engine.execute\(\) method is considered legacy .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The Executable.execute\(\) method is considered .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'The connection.execute\(\) method in SQLAlchemy 2.0 .*', -- GitLab From e1039e0849890c6bd10516004c6dab89db1c9111 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 11:36:01 +0100 Subject: [PATCH 06/16] Don't pass kwargs to connection.execute() Resolve the following RemovedIn20Warning warning: The connection.execute() method in SQLAlchemy 2.0 will accept parameters as a single dictionary or a single sequence of dictionaries only. Parameters passed as keyword arguments, tuples or positionally oriented dictionaries and/or tuples will no longer be accepted. Change-Id: I44675fce86337696b6494abc03e8058af32686c6 Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/provision.py | 10 +++++++--- oslo_db/sqlalchemy/utils.py | 2 +- oslo_db/tests/fixtures.py | 5 ----- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 3 +-- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/oslo_db/sqlalchemy/provision.py b/oslo_db/sqlalchemy/provision.py index f891862..4d1add5 100644 --- a/oslo_db/sqlalchemy/provision.py +++ b/oslo_db/sqlalchemy/provision.py @@ -605,9 +605,11 @@ class PostgresqlBackendImpl(BackendImpl): return bool( engine.scalar( sqlalchemy.text( - "SELECT datname FROM pg_database " - "WHERE datname=:name"), name=ident) + "SELECT datname FROM pg_database WHERE datname=:name" + ), + {'name': ident}, ) + ) def _close_out_database_users(self, conn, ident): """Attempt to guarantee a database can be dropped. @@ -631,7 +633,9 @@ class PostgresqlBackendImpl(BackendImpl): "WHERE usename=current_user AND " "pid != pg_backend_pid() AND " "datname=:dname" - ), dname=ident) + ), + {'dname': ident}, + ) def _random_ident(): diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 180b07f..90236d5 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -1118,7 +1118,7 @@ def get_non_innodb_tables(connectable, skip_tables=('migrate_version', params['database'] = connectable.engine.url.database query = text(query_str) - noninnodb = connectable.execute(query, **params) + noninnodb = connectable.execute(query, params) return [i[0] for i in noninnodb] diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 561ebc5..ccecccd 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -52,11 +52,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Engine.execute\(\) method is considered legacy .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The connection.execute\(\) method in SQLAlchemy 2.0 .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'Calling the mapper\(\) function directly outside .*', diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index 003a49a..45539c5 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -324,8 +324,7 @@ class MySQLModeTestCase(db_test_base._MySQLOpportunisticTestCase): def _test_string_too_long(self, value): with self.connection.begin(): - self.connection.execute(self.test_table.insert(), - bar=value) + self.connection.execute(self.test_table.insert(), {'bar': value}) result = self.connection.execute(self.test_table.select()) return result.fetchone().bar -- GitLab From ecb15c4ac63772e3332ac0f475e852f414ca3bbb Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 11:40:08 +0100 Subject: [PATCH 07/16] Don't call mapper() outside of declarative registry Resolve the following RemovedIn20Warning: Calling the mapper() function directly outside of a declarative registry is deprecated. Please use the sqlalchemy.orm.registry.map_imperatively() function for a classical mapping. Change-Id: I92a7ccdd48eedd4c788384033743daf50a9dc113 Signed-off-by: Stephen Finucane --- oslo_db/tests/fixtures.py | 5 ----- oslo_db/tests/sqlalchemy/test_enginefacade.py | 6 ++++-- oslo_db/tests/sqlalchemy/test_exc_filters.py | 7 +++++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index ccecccd..0168116 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -52,11 +52,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Engine.execute\(\) method is considered legacy .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'Calling the mapper\(\) function directly outside .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'The current statement is being autocommitted .*', diff --git a/oslo_db/tests/sqlalchemy/test_enginefacade.py b/oslo_db/tests/sqlalchemy/test_enginefacade.py index b24892b..a188d01 100644 --- a/oslo_db/tests/sqlalchemy/test_enginefacade.py +++ b/oslo_db/tests/sqlalchemy/test_enginefacade.py @@ -24,7 +24,7 @@ from oslo_context import context as oslo_context from sqlalchemy import Column from sqlalchemy import Integer from sqlalchemy import MetaData -from sqlalchemy.orm import mapper +from sqlalchemy.orm import registry from sqlalchemy.orm import Session from sqlalchemy import select from sqlalchemy import String @@ -1671,11 +1671,13 @@ class LiveFacadeTest(db_test_base._DbTestCase): metadata.create_all(self.engine) self.addCleanup(metadata.drop_all, self.engine) + reg = registry() + class User(object): def __init__(self, name): self.name = name - mapper(User, user_table) + reg.map_imperatively(User, user_table) self.User = User def _assert_ctx_connection(self, context, connection): diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index a05c9a9..27a37a4 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -23,7 +23,7 @@ from sqlalchemy.engine import url as sqla_url from sqlalchemy import event import sqlalchemy.exc from sqlalchemy.orm import declarative_base -from sqlalchemy.orm import mapper +from sqlalchemy.orm import registry from sqlalchemy import sql from oslo_db import exception @@ -1046,10 +1046,13 @@ class IntegrationTest(db_test_base._DbTestCase): self.test_table.create(self.engine) self.addCleanup(self.test_table.drop, self.engine) + reg = registry() + class Foo(object): def __init__(self, counter): self.counter = counter - mapper(Foo, self.test_table) + + reg.map_imperatively(Foo, self.test_table) self.Foo = Foo def test_flush_wrapper_duplicate_entry(self): -- GitLab From df901a1c765c16de77c2f734760e9ecf95483737 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 19 Jul 2021 11:48:58 +0100 Subject: [PATCH 08/16] Replace use of 'Engine.execute()' Resolve the following RemovedIn20Warning warning: The Engine.execute() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. All statement execution in SQLAlchemy 2.0 is performed by the Connection.execute() method of Connection, or in the ORM by the Session.execute() method of Session. Change-Id: I4c47a690a94abcb3b4b6fb087a1bf86c5350b523 Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/utils.py | 58 +++---- oslo_db/tests/fixtures.py | 11 +- oslo_db/tests/sqlalchemy/test_exc_filters.py | 153 ++++++++++--------- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 91 ++++++----- oslo_db/tests/sqlalchemy/test_utils.py | 79 +++++----- 5 files changed, 217 insertions(+), 175 deletions(-) diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 90236d5..333fff0 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -490,32 +490,34 @@ def drop_old_duplicate_entries_from_table(engine, table_name, func.count(table.c.id) > 1 ) - for row in engine.execute(duplicated_rows_select).fetchall(): - # NOTE(boris-42): Do not remove row that has the biggest ID. - delete_condition = table.c.id != row[0] - is_none = None # workaround for pyflakes - delete_condition &= table.c.deleted_at == is_none - for name in uc_column_names: - delete_condition &= table.c[name] == row._mapping[name] - - rows_to_delete_select = sqlalchemy.sql.select( - table.c.id, - ).where(delete_condition) - for row in engine.execute(rows_to_delete_select).fetchall(): - LOG.info("Deleting duplicated row with id: %(id)s from table: " - "%(table)s", dict(id=row[0], table=table_name)) - - if use_soft_delete: - delete_statement = table.update().\ - where(delete_condition).\ - values({ - 'deleted': literal_column('id'), - 'updated_at': literal_column('updated_at'), - 'deleted_at': timeutils.utcnow() - }) - else: - delete_statement = table.delete().where(delete_condition) - engine.execute(delete_statement) + with engine.connect() as conn: + for row in conn.execute(duplicated_rows_select).fetchall(): + # NOTE(boris-42): Do not remove row that has the biggest ID. + delete_condition = table.c.id != row[0] + is_none = None # workaround for pyflakes + delete_condition &= table.c.deleted_at == is_none + for name in uc_column_names: + delete_condition &= table.c[name] == row._mapping[name] + + rows_to_delete_select = sqlalchemy.sql.select( + table.c.id, + ).where(delete_condition) + for row in conn.execute(rows_to_delete_select).fetchall(): + LOG.info( + "Deleting duplicated row with id: %(id)s from table: " + "%(table)s", dict(id=row[0], table=table_name)) + + if use_soft_delete: + delete_statement = table.update().\ + where(delete_condition).\ + values({ + 'deleted': literal_column('id'), + 'updated_at': literal_column('updated_at'), + 'deleted_at': timeutils.utcnow() + }) + else: + delete_statement = table.delete().where(delete_condition) + conn.execute(delete_statement) def _get_default_deleted_value(table): @@ -1118,7 +1120,9 @@ def get_non_innodb_tables(connectable, skip_tables=('migrate_version', params['database'] = connectable.engine.url.database query = text(query_str) - noninnodb = connectable.execute(query, params) + # TODO(stephenfin): What about if this is already a Connection? + with connectable.connect() as conn: + noninnodb = conn.execute(query, params) return [i[0] for i in noninnodb] diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 0168116..521639c 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -47,11 +47,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Session.begin.subtransactions flag is deprecated .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The Engine.execute\(\) method is considered legacy .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'The current statement is being autocommitted .*', @@ -73,4 +68,10 @@ class WarningsFixture(fixtures.Fixture): module='migrate', category=sqla_exc.SADeprecationWarning) + warnings.filterwarnings( + 'ignore', + message=r'The Engine.execute\(\) method is considered legacy .*', + module='migrate', + category=sqla_exc.SADeprecationWarning) + self.addCleanup(warnings.resetwarnings) diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index 27a37a4..528d3ed 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -270,14 +270,16 @@ class TestNonExistentConstraintPostgreSQL( ): def test_raise(self): - matched = self.assertRaises( - exception.DBNonExistentConstraint, - self.engine.execute, - sqla.schema.DropConstraint( - sqla.ForeignKeyConstraint(["id"], ["baz.id"], - name="bar_fkey", - table=self.table_1)), - ) + with self.engine.connect() as conn: + matched = self.assertRaises( + exception.DBNonExistentConstraint, + conn.execute, + sqla.schema.DropConstraint( + sqla.ForeignKeyConstraint(["id"], ["baz.id"], + name="bar_fkey", + table=self.table_1)), + ) + self.assertInnerException( matched, "ProgrammingError", @@ -295,14 +297,16 @@ class TestNonExistentConstraintMySQL( ): def test_raise(self): - matched = self.assertRaises( - exception.DBNonExistentConstraint, - self.engine.execute, - sqla.schema.DropConstraint( - sqla.ForeignKeyConstraint(["id"], ["baz.id"], - name="bar_fkey", - table=self.table_1)), - ) + with self.engine.connect() as conn: + matched = self.assertRaises( + exception.DBNonExistentConstraint, + conn.execute, + sqla.schema.DropConstraint( + sqla.ForeignKeyConstraint(["id"], ["baz.id"], + name="bar_fkey", + table=self.table_1)), + ) + # NOTE(jd) Cannot check precisely with assertInnerException since MySQL # error are not the same depending on its version… self.assertIsInstance(matched.inner_exception, @@ -332,11 +336,13 @@ class TestNonExistentTable( ) def test_raise(self): - matched = self.assertRaises( - exception.DBNonExistentTable, - self.engine.execute, - sqla.schema.DropTable(self.table_1), - ) + with self.engine.connect() as conn: + matched = self.assertRaises( + exception.DBNonExistentTable, + conn.execute, + sqla.schema.DropTable(self.table_1), + ) + self.assertInnerException( matched, "OperationalError", @@ -352,11 +358,13 @@ class TestNonExistentTablePostgreSQL( ): def test_raise(self): - matched = self.assertRaises( - exception.DBNonExistentTable, - self.engine.execute, - sqla.schema.DropTable(self.table_1), - ) + with self.engine.connect() as conn: + matched = self.assertRaises( + exception.DBNonExistentTable, + conn.execute, + sqla.schema.DropTable(self.table_1), + ) + self.assertInnerException( matched, "ProgrammingError", @@ -372,11 +380,13 @@ class TestNonExistentTableMySQL( ): def test_raise(self): - matched = self.assertRaises( - exception.DBNonExistentTable, - self.engine.execute, - sqla.schema.DropTable(self.table_1), - ) + with self.engine.connect() as conn: + matched = self.assertRaises( + exception.DBNonExistentTable, + conn.execute, + sqla.schema.DropTable(self.table_1), + ) + # NOTE(jd) Cannot check precisely with assertInnerException since MySQL # error are not the same depending on its version… self.assertIsInstance(matched.inner_exception, @@ -488,13 +498,14 @@ class TestReferenceErrorSQLite( self.table_2.create(self.engine) def test_raise(self): - self.engine.execute(sql.text("PRAGMA foreign_keys = ON")) + with self.engine.connect() as conn: + conn.execute(sql.text("PRAGMA foreign_keys = ON")) - matched = self.assertRaises( - exception.DBReferenceError, - self.engine.execute, - self.table_2.insert().values(id=1, foo_id=2) - ) + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_2.insert().values(id=1, foo_id=2) + ) self.assertInnerException( matched, @@ -510,16 +521,17 @@ class TestReferenceErrorSQLite( self.assertIsNone(matched.key_table) def test_raise_delete(self): - self.engine.execute(sql.text("PRAGMA foreign_keys = ON")) - with self.engine.connect() as conn: + conn.execute(sql.text("PRAGMA foreign_keys = ON")) conn.execute(self.table_1.insert().values(id=1234, foo=42)) conn.execute(self.table_2.insert().values(id=4321, foo_id=1234)) - matched = self.assertRaises( - exception.DBReferenceError, - self.engine.execute, - self.table_1.delete() - ) + + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_1.delete() + ) + self.assertInnerException( matched, "IntegrityError", @@ -539,12 +551,14 @@ class TestReferenceErrorPostgreSQL( db_test_base._PostgreSQLOpportunisticTestCase, ): def test_raise(self): - params = {'id': 1, 'foo_id': 2} - matched = self.assertRaises( - exception.DBReferenceError, - self.engine.execute, - self.table_2.insert().values(**params) - ) + with self.engine.connect() as conn: + params = {'id': 1, 'foo_id': 2} + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_2.insert().values(**params) + ) + self.assertInnerException( matched, "IntegrityError", @@ -565,11 +579,12 @@ class TestReferenceErrorPostgreSQL( with self.engine.connect() as conn: conn.execute(self.table_1.insert().values(id=1234, foo=42)) conn.execute(self.table_2.insert().values(id=4321, foo_id=1234)) - matched = self.assertRaises( - exception.DBReferenceError, - self.engine.execute, - self.table_1.delete() - ) + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_1.delete() + ) + self.assertInnerException( matched, "IntegrityError", @@ -592,11 +607,12 @@ class TestReferenceErrorMySQL( db_test_base._MySQLOpportunisticTestCase, ): def test_raise(self): - matched = self.assertRaises( - exception.DBReferenceError, - self.engine.execute, - self.table_2.insert().values(id=1, foo_id=2) - ) + with self.engine.connect() as conn: + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_2.insert().values(id=1, foo_id=2) + ) # NOTE(jd) Cannot check precisely with assertInnerException since MySQL # error are not the same depending on its version… @@ -635,11 +651,11 @@ class TestReferenceErrorMySQL( with self.engine.connect() as conn: conn.execute(self.table_1.insert().values(id=1234, foo=42)) conn.execute(self.table_2.insert().values(id=4321, foo_id=1234)) - matched = self.assertRaises( - exception.DBReferenceError, - self.engine.execute, - self.table_1.delete() - ) + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_1.delete() + ) # NOTE(jd) Cannot check precisely with assertInnerException since MySQL # error are not the same depending on its version… self.assertIsInstance(matched.inner_exception, @@ -1185,9 +1201,10 @@ class TestDBDisconnected(TestsExceptionFilter): # test implicit execution with self._fixture(dialect_name, exc_obj, 1): - self.assertEqual( - 1, self.engine.execute(sqla.select(1)).scalars().first(), - ) + with self.engine.connect() as conn: + self.assertEqual( + 1, conn.execute(sqla.select(1)).scalars().first(), + ) def test_mariadb_error_1927(self): for code in [1927]: diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index 45539c5..de2a6dc 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -109,7 +109,7 @@ class SQLiteSavepointTest(db_test_base._DbTestCase): ) self.assertEqual( [(1, 'data 1')], - self.engine.execute( + conn.execute( self.test_table.select(). order_by(self.test_table.c.id) ).fetchall() @@ -145,13 +145,13 @@ class SQLiteSavepointTest(db_test_base._DbTestCase): {'data': 'data 3'} ) - self.assertEqual( - [(1, 'data 1'), (2, 'data 3')], - self.engine.execute( - self.test_table.select(). - order_by(self.test_table.c.id) - ).fetchall() - ) + self.assertEqual( + [(1, 'data 1'), (2, 'data 3')], + conn.execute( + self.test_table.select(). + order_by(self.test_table.c.id) + ).fetchall() + ) def test_savepoint_beginning(self): with self.engine.begin() as conn: @@ -167,13 +167,13 @@ class SQLiteSavepointTest(db_test_base._DbTestCase): {'data': 'data 2'} ) - self.assertEqual( - [(1, 'data 2')], - self.engine.execute( - self.test_table.select(). - order_by(self.test_table.c.id) - ).fetchall() - ) + self.assertEqual( + [(1, 'data 2')], + conn.execute( + self.test_table.select(). + order_by(self.test_table.c.id) + ).fetchall() + ) class FakeDBAPIConnection(object): @@ -476,34 +476,42 @@ class SQLiteConnectTest(test_base.BaseTestCase): def test_sqlite_fk_listener(self): engine = self._fixture(sqlite_fk=True) - self.assertEqual( - 1, - engine.execute(sql.text('pragma foreign_keys')).scalars().first(), - ) + with engine.connect() as conn: + self.assertEqual( + 1, + conn.execute( + sql.text('pragma foreign_keys') + ).scalars().first(), + ) engine = self._fixture(sqlite_fk=False) - self.assertEqual( - 0, - engine.execute(sql.text('pragma foreign_keys')).scalars().first(), - ) + with engine.connect() as conn: + self.assertEqual( + 0, + conn.execute( + sql.text('pragma foreign_keys') + ).scalars().first(), + ) def test_sqlite_synchronous_listener(self): engine = self._fixture() # "The default setting is synchronous=FULL." (e.g. 2) # http://www.sqlite.org/pragma.html#pragma_synchronous - self.assertEqual( - 2, - engine.execute(sql.text('pragma synchronous')).scalars().first(), - ) + with engine.connect() as conn: + self.assertEqual( + 2, + conn.execute(sql.text('pragma synchronous')).scalars().first(), + ) engine = self._fixture(sqlite_synchronous=False) - self.assertEqual( - 0, - engine.execute(sql.text('pragma synchronous')).scalars().first(), - ) + with engine.connect() as conn: + self.assertEqual( + 0, + conn.execute(sql.text('pragma synchronous')).scalars().first(), + ) class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase): @@ -512,9 +520,10 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase): return session.create_engine(self.engine.url, mysql_sql_mode=sql_mode) def _assert_sql_mode(self, engine, sql_mode_present, sql_mode_non_present): - mode = engine.execute( - sql.text("SHOW VARIABLES LIKE 'sql_mode'") - ).fetchone()[1] + with engine.connect() as conn: + mode = conn.execute( + sql.text("SHOW VARIABLES LIKE 'sql_mode'") + ).fetchone()[1] self.assertIn( sql_mode_present, mode ) @@ -538,9 +547,10 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase): # get the GLOBAL sql_mode, not the @@SESSION, so that # we get what is configured for the MySQL database, as opposed # to what our own session.create_engine() has set it to. - expected = self.engine.execute( - sql.text("SELECT @@GLOBAL.sql_mode") - ).scalar() + with self.engine.connect() as conn: + expected = conn.execute( + sql.text("SELECT @@GLOBAL.sql_mode") + ).scalar() engine = self._fixture(sql_mode=None) self._assert_sql_mode(engine, expected, None) @@ -592,9 +602,10 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase): engine = self._fixture(sql_mode='TRADITIONAL') - actual_mode = engine.execute( - sql.text("SHOW VARIABLES LIKE 'sql_mode'") - ).fetchone()[1] + with engine.connect() as conn: + actual_mode = conn.execute( + sql.text("SHOW VARIABLES LIKE 'sql_mode'") + ).fetchone()[1] self.assertIn('MySQL server mode set to %s' % actual_mode, log.output) diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 2f61004..2cbf47d 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -699,7 +699,8 @@ class TestMigrationUtils(db_test_base._DbTestCase): Column('updated_at', DateTime)) test_table.create(engine) - engine.execute(test_table.insert(), values) + with engine.connect() as conn: + conn.execute(test_table.insert(), values) return test_table, values def test_drop_old_duplicate_entries_from_table(self): @@ -719,10 +720,11 @@ class TestMigrationUtils(db_test_base._DbTestCase): uniq_values.add(uniq_value) expected_ids.append(value['id']) - real_ids = [ - row[0] for row in - self.engine.execute(select(test_table.c.id)).fetchall() - ] + with self.engine.connect() as conn: + real_ids = [ + row[0] for row in + conn.execute(select(test_table.c.id)).fetchall() + ] self.assertEqual(len(expected_ids), len(real_ids)) for id_ in expected_ids: @@ -760,20 +762,21 @@ class TestMigrationUtils(db_test_base._DbTestCase): base_select = table.select() - rows_select = base_select.where(table.c.deleted != table.c.id) - row_ids = [ - row.id for row in self.engine.execute(rows_select).fetchall() - ] - self.assertEqual(len(expected_values), len(row_ids)) - for value in expected_values: - self.assertIn(value['id'], row_ids) - - deleted_rows_select = base_select.where( - table.c.deleted == table.c.id) - deleted_rows_ids = [ - row.id for row in - self.engine.execute(deleted_rows_select).fetchall() - ] + with self.engine.connect() as conn: + rows_select = base_select.where(table.c.deleted != table.c.id) + row_ids = [ + row.id for row in conn.execute(rows_select).fetchall() + ] + self.assertEqual(len(expected_values), len(row_ids)) + for value in expected_values: + self.assertIn(value['id'], row_ids) + + deleted_rows_select = base_select.where( + table.c.deleted == table.c.id) + deleted_rows_ids = [ + row.id for row in + conn.execute(deleted_rows_select).fetchall() + ] self.assertEqual(len(values) - len(row_ids), len(deleted_rows_ids)) for value in soft_deleted_values: @@ -1649,43 +1652,49 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase): class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase): def test_all_tables_use_innodb(self): - self.engine.execute( - sql.text( - "CREATE TABLE customers " - "(a INT, b CHAR (20), INDEX (a)) ENGINE=InnoDB")) + with self.engine.connect() as conn: + conn.execute( + sql.text( + "CREATE TABLE customers " + "(a INT, b CHAR (20), INDEX (a)) ENGINE=InnoDB")) self.assertEqual([], utils.get_non_innodb_tables(self.engine)) def test_all_tables_use_innodb_false(self): - self.engine.execute( - sql.text("CREATE TABLE employee (i INT) ENGINE=MEMORY")) + with self.engine.connect() as conn: + conn.execute( + sql.text("CREATE TABLE employee (i INT) ENGINE=MEMORY")) self.assertEqual(['employee'], utils.get_non_innodb_tables(self.engine)) def test_skip_tables_use_default_value(self): - self.engine.execute( - sql.text("CREATE TABLE migrate_version (i INT) ENGINE=MEMORY")) + with self.engine.connect() as conn: + conn.execute( + sql.text("CREATE TABLE migrate_version (i INT) ENGINE=MEMORY")) self.assertEqual([], utils.get_non_innodb_tables(self.engine)) def test_skip_tables_use_passed_value(self): - self.engine.execute( - sql.text("CREATE TABLE some_table (i INT) ENGINE=MEMORY")) + with self.engine.connect() as conn: + conn.execute( + sql.text("CREATE TABLE some_table (i INT) ENGINE=MEMORY")) self.assertEqual([], utils.get_non_innodb_tables( self.engine, skip_tables=('some_table',))) def test_skip_tables_use_empty_list(self): - self.engine.execute( - sql.text("CREATE TABLE some_table_3 (i INT) ENGINE=MEMORY")) + with self.engine.connect() as conn: + conn.execute( + sql.text("CREATE TABLE some_table_3 (i INT) ENGINE=MEMORY")) self.assertEqual(['some_table_3'], utils.get_non_innodb_tables( self.engine, skip_tables=())) def test_skip_tables_use_several_values(self): - self.engine.execute( - sql.text("CREATE TABLE some_table_1 (i INT) ENGINE=MEMORY")) - self.engine.execute( - sql.text("CREATE TABLE some_table_2 (i INT) ENGINE=MEMORY")) + with self.engine.connect() as conn: + conn.execute( + sql.text("CREATE TABLE some_table_1 (i INT) ENGINE=MEMORY")) + conn.execute( + sql.text("CREATE TABLE some_table_2 (i INT) ENGINE=MEMORY")) self.assertEqual([], utils.get_non_innodb_tables( self.engine, -- GitLab From b3a56b35640b0cfceeeb97c885318a3aae7f7b22 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 10 Aug 2021 17:42:47 +0100 Subject: [PATCH 09/16] Don't rely on implicit autocommit Resolve the following RemovedIn20Warning warning: The current statement is being autocommitted using implicit autocommit, which will be removed in SQLAlchemy 2.0. Use the .begin() method of Engine or Connection in order to use an explicit transaction for DML and DDL statements. Change-Id: Ib789cd4d11a3d5dd01fcdb99822025b11bbc234e Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/provision.py | 24 ++-- oslo_db/sqlalchemy/utils.py | 124 +++++++++++-------- oslo_db/tests/fixtures.py | 11 +- oslo_db/tests/sqlalchemy/test_exc_filters.py | 56 ++++++--- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 7 +- oslo_db/tests/sqlalchemy/test_utils.py | 29 +++-- 6 files changed, 148 insertions(+), 103 deletions(-) diff --git a/oslo_db/sqlalchemy/provision.py b/oslo_db/sqlalchemy/provision.py index 4d1add5..4bce257 100644 --- a/oslo_db/sqlalchemy/provision.py +++ b/oslo_db/sqlalchemy/provision.py @@ -518,12 +518,14 @@ class MySQLBackendImpl(BackendImpl): def create_named_database(self, engine, ident, conditional=False): with engine.connect() as conn: if not conditional or not self.database_exists(conn, ident): - conn.exec_driver_sql("CREATE DATABASE %s" % ident) + with conn.begin(): + conn.exec_driver_sql("CREATE DATABASE %s" % ident) def drop_named_database(self, engine, ident, conditional=False): with engine.connect() as conn: if not conditional or self.database_exists(conn, ident): - conn.exec_driver_sql("DROP DATABASE %s" % ident) + with conn.begin(): + conn.exec_driver_sql("DROP DATABASE %s" % ident) def database_exists(self, engine, ident): s = sql.text("SHOW DATABASES LIKE :ident") @@ -582,18 +584,22 @@ class PostgresqlBackendImpl(BackendImpl): def create_named_database(self, engine, ident, conditional=False): with engine.connect().execution_options( - isolation_level="AUTOCOMMIT") as conn: + isolation_level="AUTOCOMMIT", + ) as conn: if not conditional or not self.database_exists(conn, ident): - conn.exec_driver_sql("CREATE DATABASE %s" % ident) + with conn.begin(): + conn.exec_driver_sql("CREATE DATABASE %s" % ident) def drop_named_database(self, engine, ident, conditional=False): with engine.connect().execution_options( - isolation_level="AUTOCOMMIT") as conn: + isolation_level="AUTOCOMMIT", + ) as conn: self._close_out_database_users(conn, ident) - if conditional: - conn.exec_driver_sql("DROP DATABASE IF EXISTS %s" % ident) - else: - conn.exec_driver_sql("DROP DATABASE %s" % ident) + with conn.begin(): + if conditional: + conn.exec_driver_sql("DROP DATABASE IF EXISTS %s" % ident) + else: + conn.exec_driver_sql("DROP DATABASE %s" % ident) def drop_additional_objects(self, conn): enums = [e['name'] for e in sqlalchemy.inspect(conn).get_enums()] diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 333fff0..3c58bd6 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -490,7 +490,7 @@ def drop_old_duplicate_entries_from_table(engine, table_name, func.count(table.c.id) > 1 ) - with engine.connect() as conn: + with engine.connect() as conn, conn.begin(): for row in conn.execute(duplicated_rows_select).fetchall(): # NOTE(boris-42): Do not remove row that has the biggest ID. delete_condition = table.c.id != row[0] @@ -571,7 +571,7 @@ def change_deleted_column_type_to_boolean(engine, table_name, finally: table.metadata.bind = None - with engine.connect() as conn: + with engine.connect() as conn, conn.begin(): conn.execute( table.update().where( table.c.deleted == table.c.id @@ -615,7 +615,9 @@ def _change_deleted_column_type_to_boolean_sqlite(engine, table_name, new_table = Table( table_name + "__tmp__", meta, *(columns + constraints)) - new_table.create(conn) + + with conn.begin(): + new_table.create(conn) indexes = [] for index in get_indexes(engine, table_name): @@ -631,9 +633,10 @@ def _change_deleted_column_type_to_boolean_sqlite(engine, table_name, else: c_select.append(table.c.deleted == table.c.id) - table.drop(conn) - for index in indexes: - index.create(conn) + with conn.begin(): + table.drop(conn) + for index in indexes: + index.create(conn) table.metadata.bind = engine try: @@ -641,11 +644,12 @@ def _change_deleted_column_type_to_boolean_sqlite(engine, table_name, finally: table.metadata.bind = None - conn.execute( - new_table.update().where( - new_table.c.deleted == new_table.c.id - ).values(deleted=True) - ) + with conn.begin(): + conn.execute( + new_table.update().where( + new_table.c.deleted == new_table.c.id + ).values(deleted=True) + ) @debtcollector.removals.remove( @@ -672,17 +676,18 @@ def change_deleted_column_type_to_id_type(engine, table_name, table.metadata.bind = engine try: - with engine.connect() as conn: + with engine.connect() as conn, conn.begin(): deleted = True # workaround for pyflakes conn.execute( table.update().where( table.c.deleted == deleted ).values(new_deleted=table.c.id) ) - table.c.deleted.drop() - table.c.new_deleted.alter(name="deleted") - _restore_indexes_on_deleted_columns(conn, table_name, indexes) + table.c.deleted.drop() + table.c.new_deleted.alter(name="deleted") + + _restore_indexes_on_deleted_columns(engine, table_name, indexes) finally: table.metadata.bind = None @@ -739,10 +744,13 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, constraints.append(constraint._copy()) with engine.connect() as conn: - new_table = Table( - table_name + "__tmp__", meta, - *(columns + constraints)) - new_table.create(conn) + # we need separate transactions, since we must create the table before + # we can copy entries into it (later) + with conn.begin(): + new_table = Table( + table_name + "__tmp__", meta, + *(columns + constraints)) + new_table.create(conn) indexes = [] for index in get_indexes(engine, table_name): @@ -751,30 +759,32 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, Index(index["name"], *column_names, unique=index["unique"]) ) - table.drop(conn) - for index in indexes: - index.create(conn) + with conn.begin(): + table.drop(conn) + for index in indexes: + index.create(conn) - new_table.metadata.bind = engine - try: - new_table.rename(table_name) - finally: - new_table.metadata.bind = None + with conn.begin(): + new_table.metadata.bind = engine + try: + new_table.rename(table_name) + finally: + new_table.metadata.bind = None - deleted = True # workaround for pyflakes - conn.execute( - new_table.update().where( - new_table.c.deleted == deleted - ).values(deleted=new_table.c.id) - ) + deleted = True # workaround for pyflakes + conn.execute( + new_table.update().where( + new_table.c.deleted == deleted + ).values(deleted=new_table.c.id) + ) - # NOTE(boris-42): Fix value of deleted column: False -> "" or 0. - deleted = False # workaround for pyflakes - conn.execute( - new_table.update().where( - new_table.c.deleted == deleted - ).values(deleted=default_deleted_value) - ) + # NOTE(boris-42): Fix value of deleted column: False -> "" or 0. + deleted = False # workaround for pyflakes + conn.execute( + new_table.update().where( + new_table.c.deleted == deleted + ).values(deleted=default_deleted_value) + ) def get_db_connection_info(conn_pieces): @@ -1121,7 +1131,7 @@ def get_non_innodb_tables(connectable, skip_tables=('migrate_version', params['database'] = connectable.engine.url.database query = text(query_str) # TODO(stephenfin): What about if this is already a Connection? - with connectable.connect() as conn: + with connectable.connect() as conn, conn.begin(): noninnodb = conn.execute(query, params) return [i[0] for i in noninnodb] @@ -1232,21 +1242,25 @@ def suspend_fk_constraints_for_col_alter( ctx = MigrationContext.configure(conn) op = Operations(ctx) - for fk in fks: - op.drop_constraint( - fk['name'], fk['source_table'], type_="foreignkey") + with conn.begin(): + for fk in fks: + op.drop_constraint( + fk['name'], fk['source_table'], type_="foreignkey") + yield - for fk in fks: - op.create_foreign_key( - fk['name'], fk['source_table'], - fk['referred_table'], - fk['constrained_columns'], - fk['referred_columns'], - onupdate=fk['options'].get('onupdate'), - ondelete=fk['options'].get('ondelete'), - deferrable=fk['options'].get('deferrable'), - initially=fk['options'].get('initially'), - ) + + with conn.begin(): + for fk in fks: + op.create_foreign_key( + fk['name'], fk['source_table'], + fk['referred_table'], + fk['constrained_columns'], + fk['referred_columns'], + onupdate=fk['options'].get('onupdate'), + ondelete=fk['options'].get('ondelete'), + deferrable=fk['options'].get('deferrable'), + initially=fk['options'].get('initially'), + ) class NonCommittingConnectable(object): diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 521639c..502719b 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -47,11 +47,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Session.begin.subtransactions flag is deprecated .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The current statement is being autocommitted .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'Calling \.begin\(\) when a transaction is already .*', @@ -68,6 +63,12 @@ class WarningsFixture(fixtures.Fixture): module='migrate', category=sqla_exc.SADeprecationWarning) + warnings.filterwarnings( + 'once', + message=r'The current statement is being autocommitted .*', + module='migrate', + category=sqla_exc.SADeprecationWarning) + warnings.filterwarnings( 'ignore', message=r'The Engine.execute\(\) method is considered legacy .*', diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index 528d3ed..53789f5 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -498,9 +498,15 @@ class TestReferenceErrorSQLite( self.table_2.create(self.engine) def test_raise(self): - with self.engine.connect() as conn: - conn.execute(sql.text("PRAGMA foreign_keys = ON")) + connection = self.engine.raw_connection() + try: + cursor = connection.cursor() + cursor.execute('PRAGMA foreign_keys = ON') + cursor.close() + finally: + connection.close() + with self.engine.connect() as conn: matched = self.assertRaises( exception.DBReferenceError, conn.execute, @@ -521,16 +527,24 @@ class TestReferenceErrorSQLite( self.assertIsNone(matched.key_table) def test_raise_delete(self): - with self.engine.connect() as conn: - conn.execute(sql.text("PRAGMA foreign_keys = ON")) - conn.execute(self.table_1.insert().values(id=1234, foo=42)) - conn.execute(self.table_2.insert().values(id=4321, foo_id=1234)) + connection = self.engine.raw_connection() + try: + cursor = connection.cursor() + cursor.execute('PRAGMA foreign_keys = ON') + cursor.close() + finally: + connection.close() - matched = self.assertRaises( - exception.DBReferenceError, - conn.execute, - self.table_1.delete() - ) + with self.engine.connect() as conn: + with conn.begin(): + conn.execute(self.table_1.insert().values(id=1234, foo=42)) + conn.execute( + self.table_2.insert().values(id=4321, foo_id=1234)) + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_1.delete() + ) self.assertInnerException( matched, @@ -577,13 +591,17 @@ class TestReferenceErrorPostgreSQL( def test_raise_delete(self): with self.engine.connect() as conn: - conn.execute(self.table_1.insert().values(id=1234, foo=42)) - conn.execute(self.table_2.insert().values(id=4321, foo_id=1234)) - matched = self.assertRaises( - exception.DBReferenceError, - conn.execute, - self.table_1.delete() - ) + with conn.begin(): + conn.execute(self.table_1.insert().values(id=1234, foo=42)) + conn.execute( + self.table_2.insert().values(id=4321, foo_id=1234)) + + with conn.begin(): + matched = self.assertRaises( + exception.DBReferenceError, + conn.execute, + self.table_1.delete() + ) self.assertInnerException( matched, @@ -648,7 +666,7 @@ class TestReferenceErrorMySQL( self.assertEqual("resource_foo", matched.key_table) def test_raise_delete(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute(self.table_1.insert().values(id=1234, foo=42)) conn.execute(self.table_2.insert().values(id=4321, foo_id=1234)) matched = self.assertRaises( diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index de2a6dc..7b634f1 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -314,12 +314,15 @@ class MySQLModeTestCase(db_test_base._MySQLOpportunisticTestCase): self.test_table = Table(_TABLE_NAME + "mode", meta, Column('id', Integer, primary_key=True), Column('bar', String(255))) - self.test_table.create(self.connection) + with self.connection.begin(): + self.test_table.create(self.connection) def cleanup(): - self.test_table.drop(self.connection) + with self.connection.begin(): + self.test_table.drop(self.connection) self.connection.close() mode_engine.dispose() + self.addCleanup(cleanup) def _test_string_too_long(self, value): diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 2cbf47d..087f7ec 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -699,8 +699,9 @@ class TestMigrationUtils(db_test_base._DbTestCase): Column('updated_at', DateTime)) test_table.create(engine) - with engine.connect() as conn: - conn.execute(test_table.insert(), values) + with engine.connect() as conn, conn.begin(): + with conn.begin(): + conn.execute(test_table.insert(), values) return test_table, values def test_drop_old_duplicate_entries_from_table(self): @@ -720,7 +721,7 @@ class TestMigrationUtils(db_test_base._DbTestCase): uniq_values.add(uniq_value) expected_ids.append(value['id']) - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): real_ids = [ row[0] for row in conn.execute(select(test_table.c.id)).fetchall() @@ -762,7 +763,7 @@ class TestMigrationUtils(db_test_base._DbTestCase): base_select = table.select() - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): rows_select = base_select.where(table.c.deleted != table.c.id) row_ids = [ row.id for row in conn.execute(rows_select).fetchall() @@ -938,7 +939,7 @@ class TestMigrationUtils(db_test_base._DbTestCase): # NOTE(zzzeek): SQLAlchemy 1.2 Boolean type will disallow non 1/0 # value here, 1.1 also coerces to "1/0" so use raw SQL to test the # constraint - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.exec_driver_sql( "INSERT INTO abc (deleted) VALUES (?)", (10, ), @@ -1652,7 +1653,7 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase): class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase): def test_all_tables_use_innodb(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute( sql.text( "CREATE TABLE customers " @@ -1660,21 +1661,23 @@ class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase): self.assertEqual([], utils.get_non_innodb_tables(self.engine)) def test_all_tables_use_innodb_false(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute( - sql.text("CREATE TABLE employee (i INT) ENGINE=MEMORY")) + sql.text("CREATE TABLE employee (i INT) ENGINE=MEMORY") + ) self.assertEqual(['employee'], utils.get_non_innodb_tables(self.engine)) def test_skip_tables_use_default_value(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute( - sql.text("CREATE TABLE migrate_version (i INT) ENGINE=MEMORY")) + sql.text("CREATE TABLE migrate_version (i INT) ENGINE=MEMORY") + ) self.assertEqual([], utils.get_non_innodb_tables(self.engine)) def test_skip_tables_use_passed_value(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute( sql.text("CREATE TABLE some_table (i INT) ENGINE=MEMORY")) self.assertEqual([], @@ -1682,7 +1685,7 @@ class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase): self.engine, skip_tables=('some_table',))) def test_skip_tables_use_empty_list(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute( sql.text("CREATE TABLE some_table_3 (i INT) ENGINE=MEMORY")) self.assertEqual(['some_table_3'], @@ -1690,7 +1693,7 @@ class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase): self.engine, skip_tables=())) def test_skip_tables_use_several_values(self): - with self.engine.connect() as conn: + with self.engine.connect() as conn, conn.begin(): conn.execute( sql.text("CREATE TABLE some_table_1 (i INT) ENGINE=MEMORY")) conn.execute( -- GitLab From cd5f697a173a64df0cdb1a90ab871b81f56469d4 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 16 Jul 2021 15:41:05 +0100 Subject: [PATCH 10/16] Remove use of Session.begin.subtransactions flag Resolve the following RemovedIn20Warning warning: The Session.begin.subtransactions flag is deprecated and will be removed in SQLAlchemy version 2.0. See the documentation at session_subtransactions for background on a compatible alternative pattern. Change-Id: Ib2537bae77861ee60d17a48a72c57b88e043553e Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/models.py | 14 ++------------ oslo_db/tests/fixtures.py | 5 ----- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/oslo_db/sqlalchemy/models.py b/oslo_db/sqlalchemy/models.py index 2bad0f5..72ac7cb 100644 --- a/oslo_db/sqlalchemy/models.py +++ b/oslo_db/sqlalchemy/models.py @@ -34,18 +34,8 @@ class ModelBase(object): def save(self, session): """Save this object.""" - - # NOTE(boris-42): This part of code should be look like: - # session.add(self) - # session.flush() - # But there is a bug in sqlalchemy and eventlet that - # raises NoneType exception if there is no running - # transaction and rollback is called. As long as - # sqlalchemy has this bug we have to create transaction - # explicitly. - with session.begin(subtransactions=True): - session.add(self) - session.flush() + session.add(self) + session.flush() def __setitem__(self, key, value): setattr(self, key, value) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 502719b..35b1a87 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -42,11 +42,6 @@ class WarningsFixture(fixtures.Fixture): message=r'The Session.autocommit parameter is deprecated .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'The Session.begin.subtransactions flag is deprecated .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'Calling \.begin\(\) when a transaction is already .*', -- GitLab From 4e678589a4ad0f48a192047731dff688de802468 Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 10 Sep 2021 14:34:32 +0000 Subject: [PATCH 11/16] Update master for stable/xena Add file to the reno documentation build to show release notes for stable/xena. Use pbr instruction to increment the minor version number automatically so that master versions are higher than the versions on stable/xena. Sem-Ver: feature Change-Id: I21f4a1416a8dc316817ebc081b73b76992fca867 --- releasenotes/source/index.rst | 1 + releasenotes/source/xena.rst | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 releasenotes/source/xena.rst diff --git a/releasenotes/source/index.rst b/releasenotes/source/index.rst index 45469b2..2d9a2ab 100644 --- a/releasenotes/source/index.rst +++ b/releasenotes/source/index.rst @@ -6,6 +6,7 @@ :maxdepth: 1 unreleased + xena wallaby victoria ussuri diff --git a/releasenotes/source/xena.rst b/releasenotes/source/xena.rst new file mode 100644 index 0000000..1be85be --- /dev/null +++ b/releasenotes/source/xena.rst @@ -0,0 +1,6 @@ +========================= +Xena Series Release Notes +========================= + +.. release-notes:: + :branch: stable/xena -- GitLab From d7742015f732625ff0c3e455ac5e1c01ba3d9a45 Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 10 Sep 2021 14:34:33 +0000 Subject: [PATCH 12/16] Add Python3 yoga unit tests This is an automatically generated patch to ensure unit testing is in place for all the of the tested runtimes for yoga. See also the PTI in governance [1]. [1]: https://governance.openstack.org/tc/reference/project-testing-interface.html Change-Id: I460cc3f332c8162e197b53fc2c6ae071cadf3f78 --- .zuul.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zuul.yaml b/.zuul.yaml index 84bb45c..4066fb4 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -2,7 +2,7 @@ templates: - check-requirements - lib-forward-testing-python3 - - openstack-python3-xena-jobs + - openstack-python3-yoga-jobs - periodic-stable-jobs - publish-openstack-docs-pti - release-notes-jobs-python3 -- GitLab From 8360df736d377d02cb7f22e6c32c10295476086b Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 24 Sep 2021 11:32:46 +0100 Subject: [PATCH 13/16] Configure driver for postgres Avoid the following warning seen in e.g. nova unit tests: WARNING oslo_db.sqlalchemy.engines [-] URL postgresql://.../foo does not contain a '+drivername' portion, and will make use of a default driver. A full dbname+drivername:// protocol is recommended. The 'psycopg2' driver is chosen since this is the default for SQLAlchemy [1] and therefore what we're currently using. [1] https://docs.sqlalchemy.org/en/14/core/engines.html#postgresql Change-Id: I0f8f358a49a83366d7dec17e7bac346453f3027b Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/provision.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/oslo_db/sqlalchemy/provision.py b/oslo_db/sqlalchemy/provision.py index f891862..edb2f95 100644 --- a/oslo_db/sqlalchemy/provision.py +++ b/oslo_db/sqlalchemy/provision.py @@ -577,8 +577,7 @@ class SQLiteBackendImpl(BackendImpl): @BackendImpl.impl.dispatch_for("postgresql") class PostgresqlBackendImpl(BackendImpl): def create_opportunistic_driver_url(self): - return "postgresql://openstack_citest:openstack_citest"\ - "@localhost/postgres" + return "postgresql+psycopg2://openstack_citest:openstack_citest@localhost/postgres" # noqa: E501 def create_named_database(self, engine, ident, conditional=False): with engine.connect().execution_options( -- GitLab From a0db55dcea5caba74fd6d177038f517413b9ac69 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 19 Nov 2021 09:12:10 +0000 Subject: [PATCH 14/16] tests: Restore - don't reset - warning filters There are more various warning filters pre-configured in a typical Python environment, including a few from third-party libraries such as requests [1][2] and urllib3 [3] as well as stdlib [4]. Our fixture to configure warnings, 'WarningsFixture', called 'warnings.resetwarnings' which *reset* all the warning filters [5]. This is clearly not something we want to do, and resulted in tests puking warnings after the initial test run. Resolve this by backing up the existing warning filters before applying the filter, and then *restoring* this original list of warning filters after the test run. [1] https://github.com/psf/requests/blob/v2.26.0/requests/__init__.py#L127 [2] https://github.com/psf/requests/blob/v2.26.0/requests/__init__.py#L152 [3] https://github.com/urllib3/urllib3/blob/1.26.7/src/urllib3/__init__.py#L68-L78 [4] https://docs.python.org/3.8/library/warnings.html#default-warning-filter [5] https://docs.python.org/3.8/library/warnings.html#warnings.resetwarnings Change-Id: Ie74dad3f20002dd26fa9760c9ba452c4a40186c5 Signed-off-by: Stephen Finucane --- oslo_db/tests/fixtures.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 521639c..9b26d62 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -21,6 +21,9 @@ class WarningsFixture(fixtures.Fixture): def setUp(self): super().setUp() + + self._original_warning_filters = warnings.filters[:] + # Make deprecation warnings only happen once to avoid spamming warnings.simplefilter('once', DeprecationWarning) @@ -74,4 +77,7 @@ class WarningsFixture(fixtures.Fixture): module='migrate', category=sqla_exc.SADeprecationWarning) - self.addCleanup(warnings.resetwarnings) + self.addCleanup(self._reset_warning_filters) + + def _reset_warning_filters(self): + warnings.filters[:] = self._original_warning_filters -- GitLab From ff96dc674b88bba46ef68b9842544924820d53ab Mon Sep 17 00:00:00 2001 From: dengzhaosen Date: Tue, 21 Dec 2021 17:42:21 +0800 Subject: [PATCH 15/16] Update python testing classifier Update python testing classifier Yoga testing runtime[1] has been updated to add py39 testing as voting. Unit tests update are handled by the job template change in openstack-zuul-job - https://review.opendev.org/c/openstack/openstack-zuul-jobs/+/820286 this commit updates the classifier in setup.cfg file. [1] https://governance.openstack.org/tc/reference/runtimes/yoga.html Change-Id: I42c52cc13abeb14957676b47ce1b1b52799d499b --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 5fc1aef..7acd079 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,6 +18,7 @@ classifier = Programming Language :: Python :: 3.6 Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 + Programming Language :: Python :: 3.9 Programming Language :: Python :: 3 :: Only Programming Language :: Python :: Implementation :: CPython -- GitLab From 22c602f075795b6d5ecbbc2e229817f759613ea2 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 16 Aug 2021 13:07:19 +0100 Subject: [PATCH 16/16] utils: Remove troublesome utility methods These are not compatible with SQLAlchemy 2.0 due to their reliance on nested transactions. We should deprecate them first but doing so would push the boat out further wrt how long we have to wait before achieving compatibility with this new version. Change-Id: If3db4e8c1b681c0c62d3f04a57f92802639b3b9b Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/test_fixtures.py | 16 --- oslo_db/sqlalchemy/utils.py | 127 ------------------ oslo_db/tests/sqlalchemy/test_provision.py | 76 ----------- ...-NotCommitting-utils-fed6df0e2f85edfa.yaml | 15 +++ 4 files changed, 15 insertions(+), 219 deletions(-) create mode 100644 releasenotes/notes/remove-NotCommitting-utils-fed6df0e2f85edfa.yaml diff --git a/oslo_db/sqlalchemy/test_fixtures.py b/oslo_db/sqlalchemy/test_fixtures.py index 6b82f05..8b69d3f 100644 --- a/oslo_db/sqlalchemy/test_fixtures.py +++ b/oslo_db/sqlalchemy/test_fixtures.py @@ -254,22 +254,6 @@ class DeletesFromSchema(ResetsData): """ -class RollsBackTransaction(ResetsData): - """Fixture class that maintains a database transaction per test. - - """ - - def setup_for_reset(self, engine, facade): - conn = engine.connect() - engine = utils.NonCommittingEngine(conn) - self._reset_engine = enginefacade._TestTransactionFactory.apply_engine( - engine, facade) - - def reset_schema_data(self, engine, facade): - self._reset_engine() - engine._dispose() - - class SimpleDbFixture(BaseDbFixture): """Fixture which provides an engine from a fixed URL. diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 3c58bd6..3a6a993 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -1261,130 +1261,3 @@ def suspend_fk_constraints_for_col_alter( deferrable=fk['options'].get('deferrable'), initially=fk['options'].get('initially'), ) - - -class NonCommittingConnectable(object): - """A ``Connectable`` substitute which rolls all operations back. - - ``NonCommittingConnectable`` forms the basis of mock - ``Engine`` and ``Connection`` objects within a test. It provides - only that part of the API that should reasonably be used within - a single-connection test environment (e.g. no engine.dispose(), - connection.invalidate(), etc. ). The connection runs both within - a transaction as well as a savepoint. The transaction is there - so that any operations upon the connection can be rolled back. - If the test calls begin(), a "pseduo" transaction is returned that - won't actually commit anything. The subtransaction is there to allow - a test to successfully call rollback(), however, where all operations - to that point will be rolled back and the operations can continue, - simulating a real rollback while still remaining within a transaction - external to the test. - - """ - - _nested_trans = None - - def __init__(self, connection): - self.connection = connection - self._trans = connection.begin() - self._restart_nested() - - def _restart_nested(self): - if self._nested_trans is not None: - self._nested_trans.rollback() - self._nested_trans = self.connection.begin_nested() - - def _dispose(self): - if not self.connection.closed: - self._nested_trans.rollback() - self._trans.rollback() - self.connection.close() - - def execute(self, obj, *multiparams, **params): - """Executes the given construct and returns a :class:`.ResultProxy`.""" - - return self.connection.execute(obj, *multiparams, **params) - - def scalar(self, obj, *multiparams, **params): - """Executes and returns the first column of the first row.""" - - return self.connection.scalar(obj, *multiparams, **params) - - -class NonCommittingEngine(NonCommittingConnectable): - """``Engine`` -specific non committing connectbale.""" - - @property - def url(self): - return self.connection.engine.url - - @property - def engine(self): - return self - - def connect(self): - return NonCommittingConnection(self.connection) - - @contextlib.contextmanager - def begin(self): - conn = self.connect() - trans = conn.begin() - try: - yield conn - except Exception: - trans.rollback() - else: - trans.commit() - - -class NonCommittingConnection(NonCommittingConnectable): - """``Connection`` -specific non committing connectbale.""" - - def close(self): - """Close the 'Connection'. - - In this context, close() is a no-op. - - """ - pass - - def begin(self): - return NonCommittingTransaction(self, self.connection.begin()) - - def __enter__(self): - return self - - def __exit__(self, *arg): - pass - - -class NonCommittingTransaction(object): - """A wrapper for ``Transaction``. - - This is to accommodate being able to guaranteed start a new - SAVEPOINT when a transaction is rolled back. - - """ - def __init__(self, provisioned, transaction): - self.provisioned = provisioned - self.transaction = transaction - - def __enter__(self): - return self - - def __exit__(self, type, value, traceback): - if type is None: - try: - self.commit() - except Exception: - self.rollback() - raise - else: - self.rollback() - - def commit(self): - self.transaction.commit() - - def rollback(self): - self.transaction.rollback() - self.provisioned._restart_nested() diff --git a/oslo_db/tests/sqlalchemy/test_provision.py b/oslo_db/tests/sqlalchemy/test_provision.py index f0ef944..a6cedce 100644 --- a/oslo_db/tests/sqlalchemy/test_provision.py +++ b/oslo_db/tests/sqlalchemy/test_provision.py @@ -22,7 +22,6 @@ from oslo_db import exception from oslo_db.sqlalchemy import enginefacade from oslo_db.sqlalchemy import provision from oslo_db.sqlalchemy import test_fixtures -from oslo_db.sqlalchemy import utils from oslo_db.tests import base as test_base from oslo_db.tests.sqlalchemy import base as db_test_base @@ -149,81 +148,6 @@ class PostgreSQLDropAllObjectsTest( pass -class RetainSchemaTest(test_base.BaseTestCase): - DRIVER = "sqlite" - - def setUp(self): - super(RetainSchemaTest, self).setUp() - - metadata = schema.MetaData() - self.test_table = schema.Table( - 'test_table', metadata, - schema.Column('x', types.Integer), - schema.Column('y', types.Integer), - mysql_engine='InnoDB' - ) - - def gen_schema(engine): - metadata.create_all(engine, checkfirst=False) - self._gen_schema = gen_schema - - def test_once(self): - self._run_test() - - def test_twice(self): - self._run_test() - - def _run_test(self): - try: - database_resource = provision.DatabaseResource( - self.DRIVER, provision_new_database=True) - except exception.BackendNotAvailable: - self.skipTest("database not available") - - schema_resource = provision.SchemaResource( - database_resource, self._gen_schema) - - schema = schema_resource.getResource() - - conn = schema.database.engine.connect() - engine = utils.NonCommittingEngine(conn) - - with engine.connect() as conn: - rows = conn.execute(self.test_table.select()) - self.assertEqual([], rows.fetchall()) - - trans = conn.begin() - conn.execute( - self.test_table.insert(), - {"x": 1, "y": 2} - ) - trans.rollback() - - rows = conn.execute(self.test_table.select()) - self.assertEqual([], rows.fetchall()) - - trans = conn.begin() - conn.execute( - self.test_table.insert(), - {"x": 2, "y": 3} - ) - trans.commit() - - rows = conn.execute(self.test_table.select()) - self.assertEqual([(2, 3)], rows.fetchall()) - - engine._dispose() - schema_resource.finishedWith(schema) - - -class MySQLRetainSchemaTest(RetainSchemaTest): - DRIVER = "mysql" - - -class PostgresqlRetainSchemaTest(RetainSchemaTest): - DRIVER = "postgresql" - - class AdHocURLTest(test_base.BaseTestCase): def test_sqlite_setup_teardown(self): diff --git a/releasenotes/notes/remove-NotCommitting-utils-fed6df0e2f85edfa.yaml b/releasenotes/notes/remove-NotCommitting-utils-fed6df0e2f85edfa.yaml new file mode 100644 index 0000000..c57fbad --- /dev/null +++ b/releasenotes/notes/remove-NotCommitting-utils-fed6df0e2f85edfa.yaml @@ -0,0 +1,15 @@ +--- +upgrade: + - | + The following helpers have been removed from the + ``oslo_db.sqlalchemy.utils`` module: + + - ``NonCommittingConnectable`` + - ``NonCommittingEngine`` + - ``NonCommittingConnection`` + - ``NonCommittingTransaction`` + + These were unused outside of oslo.db and were not compatible with + SQLAlchemy 2.0. In addition, the ``RollsBackTransaction`` fixture has + been removed from ``oslo_db.sqlalchemy.test_fixtures``. This was + similarly unused and presented similar compatibility issues. -- GitLab