From 872bc1ddccf85b2a2a571838809130749737e101 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 16 Jul 2021 16:52:00 +0100 Subject: 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. Change-Id: I1faa8c957649a04aa080518651045b432c6bd372 Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/provision.py | 30 +++++++++++----------- oslo_db/tests/fixtures.py | 16 ++++++++---- oslo_db/tests/sqlalchemy/test_exc_filters.py | 9 ++++--- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 23 ++++++++++------- oslo_db/tests/sqlalchemy/test_utils.py | 37 +++++++++++++++------------- 5 files changed, 66 insertions(+), 49 deletions(-) diff --git a/oslo_db/sqlalchemy/provision.py b/oslo_db/sqlalchemy/provision.py index 199cba2..f891862 100644 --- a/oslo_db/sqlalchemy/provision.py +++ b/oslo_db/sqlalchemy/provision.py @@ -26,6 +26,7 @@ import string import sqlalchemy from sqlalchemy.engine import url as sa_url from sqlalchemy import schema +from sqlalchemy import sql import testresources from oslo_db import exception @@ -517,15 +518,16 @@ 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.execute("CREATE DATABASE %s" % ident) + 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.execute("DROP DATABASE %s" % ident) + conn.exec_driver_sql("DROP DATABASE %s" % ident) def database_exists(self, engine, ident): - return bool(engine.scalar("SHOW DATABASES LIKE '%s'" % ident)) + s = sql.text("SHOW DATABASES LIKE :ident") + return bool(engine.scalar(s, {'ident': ident})) @BackendImpl.impl.dispatch_for("sqlite") @@ -582,29 +584,29 @@ class PostgresqlBackendImpl(BackendImpl): with engine.connect().execution_options( isolation_level="AUTOCOMMIT") as conn: if not conditional or not self.database_exists(conn, ident): - conn.execute("CREATE DATABASE %s" % ident) + 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: self._close_out_database_users(conn, ident) if conditional: - conn.execute("DROP DATABASE IF EXISTS %s" % ident) + conn.exec_driver_sql("DROP DATABASE IF EXISTS %s" % ident) else: - conn.execute("DROP DATABASE %s" % ident) + 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()] for e in enums: - conn.execute("DROP TYPE %s" % e) + conn.exec_driver_sql("DROP TYPE %s" % e) def database_exists(self, engine, ident): 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): @@ -624,11 +626,11 @@ class PostgresqlBackendImpl(BackendImpl): if conn.dialect.server_version_info >= (9, 2): conn.execute( sqlalchemy.text( - "select pg_terminate_backend(pid) " - "from pg_stat_activity " - "where usename=current_user and " - "pid != pg_backend_pid() " - "and datname=:dname" + "SELECT pg_terminate_backend(pid) " + "FROM pg_stat_activity " + "WHERE usename=current_user AND " + "pid != pg_backend_pid() AND " + "datname=:dname" ), dname=ident) diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 00fdb98..5f1b299 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -102,11 +102,6 @@ class WarningsFixture(fixtures.Fixture): message=r'Calling the mapper\(\) function directly outside .*', category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( - 'once', - message=r'Passing a string to Connection.execute\(\) .*', - category=sqla_exc.SADeprecationWarning) - warnings.filterwarnings( 'once', message=r'Using plain strings to indicate SQL statements .*', @@ -137,4 +132,15 @@ class WarningsFixture(fixtures.Fixture): message=r'The ``declarative_base\(\)`` function is now .*', category=sqla_exc.SADeprecationWarning) + # ...plus things that aren't our fault + + # FIXME(stephenfin): These are caused by sqlalchemy-migrate, not us, + # and should be removed when we drop support for that library + + warnings.filterwarnings( + 'ignore', + message=r'Passing a string to Connection.execute\(\) .*', + 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 d04e100..97f85e6 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -24,6 +24,7 @@ from sqlalchemy import event import sqlalchemy.exc from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import mapper +from sqlalchemy import sql from oslo_db import exception from oslo_db.sqlalchemy import engines @@ -157,7 +158,7 @@ class TestsExceptionFilter(_SQLAExceptionMatcher, test_base.BaseTestCase): with self._fixture(dialect_name, raises, is_disconnect=is_disconnect): with self.engine.connect() as conn: matched = self.assertRaises( - expected, conn.execute, statement, params + expected, conn.execute, sql.text(statement), params ) return matched @@ -487,7 +488,7 @@ class TestReferenceErrorSQLite( self.table_2.create(self.engine) def test_raise(self): - self.engine.execute("PRAGMA foreign_keys = ON;") + self.engine.execute(sql.text("PRAGMA foreign_keys = ON")) matched = self.assertRaises( exception.DBReferenceError, @@ -509,7 +510,7 @@ class TestReferenceErrorSQLite( self.assertIsNone(matched.key_table) def test_raise_delete(self): - self.engine.execute("PRAGMA foreign_keys = ON;") + self.engine.execute(sql.text("PRAGMA foreign_keys = ON")) with self.engine.connect() as conn: conn.execute(self.table_1.insert({"id": 1234, "foo": 42})) @@ -612,7 +613,7 @@ class TestReferenceErrorMySQL( conn.detach() # will not be returned to the pool when closed # this is incompatible with some internals of the engine - conn.execute("SET SESSION sql_mode = 'ANSI';") + conn.execute(sql.text("SET SESSION sql_mode = 'ANSI';")) matched = self.assertRaises( exception.DBReferenceError, diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index b400249..f796368 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -24,6 +24,7 @@ from unittest import mock import fixtures from oslo_config import cfg import sqlalchemy +from sqlalchemy import sql from sqlalchemy import Column, MetaData, Table from sqlalchemy.engine import url from sqlalchemy import Integer, String @@ -289,7 +290,7 @@ class MySQLDefaultModeTestCase(db_test_base._MySQLOpportunisticTestCase): def test_default_is_traditional(self): with self.engine.connect() as conn: sql_mode = conn.execute( - "SHOW VARIABLES LIKE 'sql_mode'" + sql.text("SHOW VARIABLES LIKE 'sql_mode'") ).first()[1] self.assertIn("TRADITIONAL", sql_mode) @@ -479,14 +480,14 @@ class SQLiteConnectTest(test_base.BaseTestCase): engine = self._fixture(sqlite_fk=True) self.assertEqual( 1, - engine.scalar("pragma foreign_keys") + engine.scalar(sql.text('pragma foreign_keys')) ) engine = self._fixture(sqlite_fk=False) self.assertEqual( 0, - engine.scalar("pragma foreign_keys") + engine.scalar(sql.text("pragma foreign_keys")) ) def test_sqlite_synchronous_listener(self): @@ -496,14 +497,14 @@ class SQLiteConnectTest(test_base.BaseTestCase): # http://www.sqlite.org/pragma.html#pragma_synchronous self.assertEqual( 2, - engine.scalar("pragma synchronous") + engine.scalar(sql.text('pragma synchronous')) ) engine = self._fixture(sqlite_synchronous=False) self.assertEqual( 0, - engine.scalar("pragma synchronous") + engine.scalar(sql.text('pragma synchronous')) ) @@ -513,7 +514,9 @@ 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("SHOW VARIABLES LIKE 'sql_mode'").fetchone()[1] + mode = engine.execute( + sql.text("SHOW VARIABLES LIKE 'sql_mode'") + ).fetchone()[1] self.assertIn( sql_mode_present, mode ) @@ -538,7 +541,8 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase): # 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( - "SELECT @@GLOBAL.sql_mode").scalar() + sql.text("SELECT @@GLOBAL.sql_mode") + ).scalar() engine = self._fixture(sql_mode=None) self._assert_sql_mode(engine, expected, None) @@ -591,7 +595,8 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase): engine = self._fixture(sql_mode='TRADITIONAL') actual_mode = engine.execute( - "SHOW VARIABLES LIKE 'sql_mode'").fetchone()[1] + sql.text("SHOW VARIABLES LIKE 'sql_mode'") + ).fetchone()[1] self.assertIn('MySQL server mode set to %s' % actual_mode, log.output) @@ -832,7 +837,7 @@ class PatchStacktraceTest(db_test_base._DbTestCase): with mock.patch.object(engine.dialect, "do_execute") as mock_exec: mock_exec.side_effect = orig_do_exec - conn.execute("select 1;") + conn.execute(sql.text("select 1")) call = mock_exec.mock_calls[0] diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 9f5f7f4..c63d52f 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -31,6 +31,7 @@ from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import mapper from sqlalchemy.orm import Session from sqlalchemy import PrimaryKeyConstraint +from sqlalchemy import sql from sqlalchemy.sql.expression import cast from sqlalchemy.sql import select from sqlalchemy.types import UserDefinedType @@ -933,14 +934,14 @@ class TestMigrationUtils(db_test_base._DbTestCase): # value here, 1.1 also coerces to "1/0" so use raw SQL to test the # constraint with self.engine.connect() as conn: - conn.execute( + conn.exec_driver_sql( "INSERT INTO abc (deleted) VALUES (?)", - (10, ) + (10, ), ) self.assertEqual( 10, - conn.scalar("SELECT deleted FROM abc") + conn.scalar(sql.text("SELECT deleted FROM abc")), ) def test_get_foreign_key_constraint_name(self): @@ -1646,41 +1647,43 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase): class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase): def test_all_tables_use_innodb(self): - self.engine.execute("CREATE TABLE customers " - "(a INT, b CHAR (20), INDEX (a)) ENGINE=InnoDB") + self.engine.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("CREATE TABLE employee " - "(i INT) ENGINE=MEMORY") + self.engine.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("CREATE TABLE migrate_version " - "(i INT) ENGINE=MEMORY") + self.engine.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("CREATE TABLE some_table " - "(i INT) ENGINE=MEMORY") + self.engine.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("CREATE TABLE some_table_3 " - "(i INT) ENGINE=MEMORY") + self.engine.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("CREATE TABLE some_table_1 " - "(i INT) ENGINE=MEMORY") - self.engine.execute("CREATE TABLE some_table_2 " - "(i INT) ENGINE=MEMORY") + 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")) self.assertEqual([], utils.get_non_innodb_tables( self.engine, -- cgit v1.2.1