From 115c3247b486c713176139422647144108101ca3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 18 Sep 2023 10:05:25 +0100 Subject: [PATCH] [stable-only] Re-add autocommit support There are a number of projects that have yet to migrate away from autocommit. To prevent them blocking the release, temporarily re-add autocommit support. We do this only on the stable branch since we are going to need to address SQLAlchemy 2.x support in C. This is effectively a partial revert of change Ifaca67c07f008d8bc0febeecd3e200cc7ee7a4b0 but focused solely on autocommit and with additional logic in tests to prevent us trying to use autocommit with SQLAlchemy 2.x (where it's not supported). Change-Id: Ib49ed6886470284e6486581290af4e0f2482e5c5 Signed-off-by: Stephen Finucane --- oslo_db/sqlalchemy/enginefacade.py | 26 +++++++++++++++++-- oslo_db/tests/sqlalchemy/test_enginefacade.py | 15 +++++++++++ oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 4 ++- .../readd-autocommit-b070ba5a3a57dcad.yaml | 7 +++++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/readd-autocommit-b070ba5a3a57dcad.yaml diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py index 293b9883..f9d44494 100644 --- a/oslo_db/sqlalchemy/enginefacade.py +++ b/oslo_db/sqlalchemy/enginefacade.py @@ -176,6 +176,7 @@ class _TransactionFactory: } self._maker_cfg = { 'expire_on_commit': _Default(False), + '__autocommit': _Default(False), } self._transaction_ctx_cfg = { 'rollback_reader_sessions': False, @@ -446,6 +447,8 @@ class _TransactionFactory: def _maker_args_for_conf(self, conf): maker_args = self._args_for_conf(self._maker_cfg, conf) + if '__autocommit' in maker_args: + maker_args['autocommit'] = maker_args.pop('__autocommit') return maker_args def dispose_pool(self): @@ -1214,6 +1217,10 @@ class LegacyEngineFacade(object): :param sqlite_fk: enable foreign keys in SQLite :type sqlite_fk: bool + :param autocommit: use autocommit mode for created Session instances + (only supported with SQLAlchemy < 2.0) + :type autocommit: bool + :param expire_on_commit: expire session objects on commit :type expire_on_commit: bool @@ -1251,7 +1258,8 @@ class LegacyEngineFacade(object): """ def __init__(self, sql_connection, slave_connection=None, - sqlite_fk=False, expire_on_commit=False, _conf=None, + sqlite_fk=False, autocommit=False, + expire_on_commit=False, _conf=None, _factory=None, **kwargs): warnings.warn( "EngineFacade is deprecated; please use " @@ -1259,6 +1267,14 @@ class LegacyEngineFacade(object): warning.OsloDBDeprecationWarning, stacklevel=2) + if autocommit is True: + warnings.warn( + 'autocommit support has been removed in SQLAlchemy 2.0 and ' + 'should not be relied on; please rework your code to remove ' + 'reliance on this feature', + warning.OsloDBDeprecationWarning, + stacklevel=2) + if _factory: self._factory = _factory else: @@ -1266,6 +1282,7 @@ class LegacyEngineFacade(object): self._factory.configure( sqlite_fk=sqlite_fk, + __autocommit=autocommit, expire_on_commit=expire_on_commit, **kwargs ) @@ -1331,7 +1348,7 @@ class LegacyEngineFacade(object): @classmethod def from_config(cls, conf, - sqlite_fk=False, expire_on_commit=False): + sqlite_fk=False, autocommit=False, expire_on_commit=False): """Initialize EngineFacade using oslo.config config instance options. :param conf: oslo.config config instance @@ -1340,6 +1357,10 @@ class LegacyEngineFacade(object): :param sqlite_fk: enable foreign keys in SQLite :type sqlite_fk: bool + :param autocommit: use autocommit mode for created Session instances + (only supported with SQLAlchemy < 2.0) + :type autocommit: bool + :param expire_on_commit: expire session objects on commit :type expire_on_commit: bool @@ -1348,4 +1369,5 @@ class LegacyEngineFacade(object): return cls( None, sqlite_fk=sqlite_fk, + autocommit=autocommit, expire_on_commit=expire_on_commit, _conf=conf) diff --git a/oslo_db/tests/sqlalchemy/test_enginefacade.py b/oslo_db/tests/sqlalchemy/test_enginefacade.py index 0a1292fe..67f61a8d 100644 --- a/oslo_db/tests/sqlalchemy/test_enginefacade.py +++ b/oslo_db/tests/sqlalchemy/test_enginefacade.py @@ -361,10 +361,12 @@ class MockFacadeTest(test_base.BaseTestCase): maker_factories = mock.Mock(side_effect=get_maker) maker_factories( + autocommit=False, engine=engines.writer, expire_on_commit=False) if self.slave_uri: maker_factories( + autocommit=False, engine=engines.async_reader, expire_on_commit=False) @@ -1385,6 +1387,19 @@ class PatchFactoryTest(test_base.BaseTestCase): self.assertEqual(38, engine_args["max_overflow"]) self.assertNotIn("mysql_wsrep_sync_wait", engine_args) + def test_new_manager_deprecated_options(self): + normal_mgr = enginefacade.transaction_context() + normal_mgr.configure( + connection="sqlite://", + __autocommit=True, + ) + + normal_mgr._factory._start() + copied_mgr = normal_mgr.make_new_manager() + + engine_args = copied_mgr._factory._maker_args_for_conf(None) + self.assertTrue(engine_args['autocommit']) + def test_new_manager_from_options(self): """test enginefacade's defaults given a default structure from opts""" diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index dc4e743c..c2574677 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -432,7 +432,9 @@ class EngineFacadeTestCase(test_base.BaseTestCase): logging_name=mock.ANY, ) get_maker.assert_called_once_with( - engine=create_engine(), expire_on_commit=True, + engine=create_engine(), + autocommit=False, + expire_on_commit=True, ) def test_slave_connection(self): diff --git a/releasenotes/notes/readd-autocommit-b070ba5a3a57dcad.yaml b/releasenotes/notes/readd-autocommit-b070ba5a3a57dcad.yaml new file mode 100644 index 00000000..cac4779e --- /dev/null +++ b/releasenotes/notes/readd-autocommit-b070ba5a3a57dcad.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + The ability to create engine facades that used autocommit, which was + removed in 13.0.0, has been re-added temporarily to allow a longer + transition time for projects. It is still deprecated and requires + SQLAlchemy < 2.x. It will be removed again in a future release.