From 25dcacea7c389819d988f9340d93c69cb11ef298 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 19 Jul 2023 11:28:18 +0100 Subject: [PATCH] db: Don't pass strings to Connection.execute() Resolve the following RemovedIn20Warning warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0. Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. We also resolve two additional RemovedIn20Warning warnings along the way, since we're already touching this code: 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. 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: I547bd2f441085fcdc5dc4a0e58d27d11e8a8b223 --- .../train_migrate01_backend_to_store.py | 38 ++++++++++++------- glance/tests/functional/db/base.py | 2 +- glance/tests/functional/db/test_migrations.py | 35 ++++++++++------- glance/tests/unit/fixtures.py | 21 ---------- 4 files changed, 47 insertions(+), 49 deletions(-) diff --git a/glance/db/sqlalchemy/alembic_migrations/data_migrations/train_migrate01_backend_to_store.py b/glance/db/sqlalchemy/alembic_migrations/data_migrations/train_migrate01_backend_to_store.py index ab777b69cf..aec63000d1 100644 --- a/glance/db/sqlalchemy/alembic_migrations/data_migrations/train_migrate01_backend_to_store.py +++ b/glance/db/sqlalchemy/alembic_migrations/data_migrations/train_migrate01_backend_to_store.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +from sqlalchemy import sql + def has_migrations(engine): """Returns true if at least one data row can be migrated. @@ -22,16 +24,20 @@ def has_migrations(engine): Note: This method can return a false positive if data migrations are running in the background as it's being called. """ - sql_query = ("select meta_data from image_locations where " - "INSTR(meta_data, '\"backend\":') > 0") + sql_query = sql.text( + "select meta_data from image_locations where " + "INSTR(meta_data, '\"backend\":') > 0" + ) # NOTE(abhishekk): INSTR function doesn't supported in postgresql if engine.name == 'postgresql': - sql_query = ("select meta_data from image_locations where " - "POSITION('\"backend\":' IN meta_data) > 0") + sql_query = sql.text( + "select meta_data from image_locations where " + "POSITION('\"backend\":' IN meta_data) > 0" + ) - with engine.connect() as con: - metadata_backend = con.execute(sql_query) + with engine.connect() as conn, conn.begin(): + metadata_backend = conn.execute(sql_query) if metadata_backend.rowcount > 0: return True @@ -40,16 +46,20 @@ def has_migrations(engine): def migrate(engine): """Replace 'backend' with 'store' in meta_data column of image_locations""" - sql_query = ("UPDATE image_locations SET meta_data = REPLACE(meta_data, " - "'\"backend\":', '\"store\":') where INSTR(meta_data, " - " '\"backend\":') > 0") + sql_query = sql.text( + "UPDATE image_locations SET meta_data = REPLACE(meta_data, " + "'\"backend\":', '\"store\":') where INSTR(meta_data, " + " '\"backend\":') > 0" + ) # NOTE(abhishekk): INSTR function doesn't supported in postgresql if engine.name == 'postgresql': - sql_query = ("UPDATE image_locations SET meta_data = REPLACE(" - "meta_data, '\"backend\":', '\"store\":') where " - "POSITION('\"backend\":' IN meta_data) > 0") + sql_query = sql.text( + "UPDATE image_locations SET meta_data = REPLACE(" + "meta_data, '\"backend\":', '\"store\":') where " + "POSITION('\"backend\":' IN meta_data) > 0" + ) - with engine.connect() as con: - migrated_rows = con.execute(sql_query) + with engine.connect() as conn, conn.begin(): + migrated_rows = conn.execute(sql_query) return migrated_rows.rowcount diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py index 86ddecd800..75e7d62f1d 100644 --- a/glance/tests/functional/db/base.py +++ b/glance/tests/functional/db/base.py @@ -2104,7 +2104,7 @@ class DBPurgeTests(test_utils.BaseTestCase): dialect = engine.url.get_dialect() if dialect == sqlite.dialect: - connection.execute("PRAGMA foreign_keys = ON") + connection.exec_driver_sql("PRAGMA foreign_keys = ON") images = sqlalchemyutils.get_table( engine, "images") diff --git a/glance/tests/functional/db/test_migrations.py b/glance/tests/functional/db/test_migrations.py index 150af726db..83388b992f 100644 --- a/glance/tests/functional/db/test_migrations.py +++ b/glance/tests/functional/db/test_migrations.py @@ -23,6 +23,7 @@ from alembic import script as alembic_script from oslo_db.sqlalchemy import enginefacade from oslo_db.sqlalchemy import test_fixtures from oslo_db.sqlalchemy import test_migrations +from sqlalchemy import sql import sqlalchemy.types as types from glance.db.sqlalchemy import alembic_migrations @@ -154,21 +155,29 @@ class TestMysqlMigrations(test_fixtures.OpportunisticDBTestMixin, def test_mysql_innodb_tables(self): test_utils.db_sync(engine=self.engine) - total = self.engine.execute( - "SELECT COUNT(*) " - "FROM information_schema.TABLES " - "WHERE TABLE_SCHEMA='%s'" - % self.engine.url.database) + with self.engine.connect() as conn: + total = conn.execute( + sql.text( + "SELECT COUNT(*) " + "FROM information_schema.TABLES " + "WHERE TABLE_SCHEMA=:database" + ), + {'database': self.engine.url.database}, + ) self.assertGreater(total.scalar(), 0, "No tables found. Wrong schema?") - noninnodb = self.engine.execute( - "SELECT count(*) " - "FROM information_schema.TABLES " - "WHERE TABLE_SCHEMA='%s' " - "AND ENGINE!='InnoDB' " - "AND TABLE_NAME!='migrate_version'" - % self.engine.url.database) - count = noninnodb.scalar() + with self.engine.connect() as conn: + noninnodb = conn.execute( + sql.text( + "SELECT count(*) " + "FROM information_schema.TABLES " + "WHERE TABLE_SCHEMA=:database " + "AND ENGINE!='InnoDB' " + "AND TABLE_NAME!='migrate_version'" + ), + {'database': self.engine.url.database}, + ) + count = noninnodb.scalar() self.assertEqual(0, count, "%d non InnoDB tables created" % count) diff --git a/glance/tests/unit/fixtures.py b/glance/tests/unit/fixtures.py index 58ac7b8b95..e8c7991706 100644 --- a/glance/tests/unit/fixtures.py +++ b/glance/tests/unit/fixtures.py @@ -192,27 +192,6 @@ class WarningsFixture(pyfixtures.Fixture): message='Using non-integer/slice indices on Row is deprecated ', ) - warnings.filterwarnings( - 'ignore', - module='glance', - category=sqla_exc.SADeprecationWarning, - message=r'Passing a string to Connection.execute\(\) ', - ) - - warnings.filterwarnings( - 'ignore', - module='glance', - category=sqla_exc.SADeprecationWarning, - message=r'The Engine.execute\(\) method is considered legacy ', - ) - - warnings.filterwarnings( - 'ignore', - module='glance', - category=sqla_exc.SADeprecationWarning, - message=r'The Executable.execute\(\) method is considered legacy ', - ) - # Enable general SQLAlchemy warnings also to ensure we're not doing # silly stuff. It's possible that we'll need to filter things out here # with future SQLAlchemy versions, but that's a good thing