From 22c602f075795b6d5ecbbc2e229817f759613ea2 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 16 Aug 2021 13:07:19 +0100 Subject: [PATCH] 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 6b82f05a..8b69d3f2 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 3c58bd66..3a6a993e 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 f0ef9446..a6cedceb 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 00000000..c57fbadb --- /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.