openstack/oslo.db
Code Issues Proposed changes

Remove the 'Session.autocommit' parameter

Browse Source 
Resolve the following RemovedIn20Warning warning:

  The Session.autocommit parameter is deprecated and will be removed in
  SQLAlchemy version 2.0.  The Session now features "autobegin" behavior
  such that the Session.begin() method may be called if a transaction
  has not yet been started yet. See the section session_explicit_begin
  for background.

Change-Id: I7867cdcea115b13f2e45e0674bb9ef2ad138aae9
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2021-08-16 13:10:59 +01:00
parent 4314279250
commit a530cbfcf2
5 changed files with 83 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
oslo_db/sqlalchemy/enginefacade.py
View File

@ -163,7 +163,7 @@ class _TransactionFactory(object):
} }
self._maker_cfg = { self._maker_cfg = {
'expire_on_commit': _Default(False), 'expire_on_commit': _Default(False),
'__autocommit': True '__autocommit': False,
} }
self._transaction_ctx_cfg = { self._transaction_ctx_cfg = {
'rollback_reader_sessions': False, 'rollback_reader_sessions': False,
@ -1266,13 +1266,22 @@ class LegacyEngineFacade(object):
""" """
def __init__(self, sql_connection, slave_connection=None, def __init__(self, sql_connection, slave_connection=None,
sqlite_fk=False, autocommit=True, sqlite_fk=False, autocommit=False,
expire_on_commit=False, _conf=None, _factory=None, **kwargs): expire_on_commit=False, _conf=None, _factory=None, **kwargs):
warnings.warn( warnings.warn(
"EngineFacade is deprecated; please use " "EngineFacade is deprecated; please use "
"oslo_db.sqlalchemy.enginefacade", "oslo_db.sqlalchemy.enginefacade",
warning.OsloDBDeprecationWarning, warning.OsloDBDeprecationWarning,
stacklevel=2) stacklevel=2)
if autocommit is True:
warnings.warn(
'autocommit support will be 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: if _factory:
self._factory = _factory self._factory = _factory
else: else:
@ -1346,7 +1355,7 @@ class LegacyEngineFacade(object):
@classmethod @classmethod
def from_config(cls, conf, def from_config(cls, conf,
sqlite_fk=False, autocommit=True, expire_on_commit=False): sqlite_fk=False, autocommit=False, expire_on_commit=False):
"""Initialize EngineFacade using oslo.config config instance options. """Initialize EngineFacade using oslo.config config instance options.
:param conf: oslo.config config instance :param conf: oslo.config config instance

2
oslo_db/sqlalchemy/orm.py
View File

@ -57,7 +57,7 @@ class Session(sqlalchemy.orm.session.Session):
"""oslo.db-specific Session subclass.""" """oslo.db-specific Session subclass."""
def get_maker(engine, autocommit=True, expire_on_commit=False): def get_maker(engine, autocommit=False, expire_on_commit=False):
"""Return a SQLAlchemy sessionmaker using the given engine.""" """Return a SQLAlchemy sessionmaker using the given engine."""
return sqlalchemy.orm.sessionmaker(bind=engine, return sqlalchemy.orm.sessionmaker(bind=engine,
class_=Session, class_=Session,

10
oslo_db/tests/fixtures.py
View File

@ -37,15 +37,7 @@ class WarningsFixture(fixtures.Fixture):
'error', 'error',
category=sqla_exc.SADeprecationWarning) category=sqla_exc.SADeprecationWarning)
# ...but filter everything out until we get around to fixing them # ...but filter things that aren't our fault
# FIXME(stephenfin): Remove all of these
warnings.filterwarnings(
'once',
message=r'The Session.autocommit parameter is deprecated .*',
category=sqla_exc.SADeprecationWarning)
# ...plus things that aren't our fault
# FIXME(stephenfin): These are caused by sqlalchemy-migrate, not us, # FIXME(stephenfin): These are caused by sqlalchemy-migrate, not us,
# and should be removed when we drop support for that library # and should be removed when we drop support for that library

38
oslo_db/tests/sqlalchemy/test_enginefacade.py
View File

@ -357,11 +357,11 @@ class MockFacadeTest(test_base.BaseTestCase):
maker_factories = mock.Mock(side_effect=get_maker) maker_factories = mock.Mock(side_effect=get_maker)
maker_factories( maker_factories(
autocommit=True, engine=engines.writer, autocommit=False, engine=engines.writer,
expire_on_commit=False) expire_on_commit=False)
if self.slave_uri: if self.slave_uri:
maker_factories( maker_factories(
autocommit=True, engine=engines.async_reader, autocommit=False, engine=engines.async_reader,
expire_on_commit=False) expire_on_commit=False)
yield makers yield makers
@ -1692,7 +1692,8 @@ class LiveFacadeTest(db_test_base._DbTestCase):
with enginefacade.writer.using(context) as session: with enginefacade.writer.using(context) as session:
session.add(self.User(name="u1")) session.add(self.User(name="u1"))
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
with session.begin():
self.assertEqual( self.assertEqual(
"u1", "u1",
session.query(self.User.name).scalar() session.query(self.User.name).scalar()
@ -1712,7 +1713,8 @@ class LiveFacadeTest(db_test_base._DbTestCase):
self.assertRaises(MyException, go, context) self.assertRaises(MyException, go, context)
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
with session.begin():
self.assertEqual( self.assertEqual(
None, None,
session.query(self.User.name).scalar() session.query(self.User.name).scalar()
@ -1783,7 +1785,8 @@ class LiveFacadeTest(db_test_base._DbTestCase):
s2.add(self.User(name="u1")) s2.add(self.User(name="u1"))
s2.flush() s2.flush()
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
with session.begin():
self.assertEqual( self.assertEqual(
"u1", "u1",
session.query(self.User.name).scalar() session.query(self.User.name).scalar()
@ -1804,7 +1807,8 @@ class LiveFacadeTest(db_test_base._DbTestCase):
self._assert_ctx_connection(ctx2, conn2) self._assert_ctx_connection(ctx2, conn2)
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
with session.begin():
self.assertEqual( self.assertEqual(
"u1", "u1",
session.query(self.User.name).scalar() session.query(self.User.name).scalar()
@ -1840,9 +1844,9 @@ class LiveFacadeTest(db_test_base._DbTestCase):
session.begin() session.begin()
session.add(self.User(name="u4")) session.add(self.User(name="u4"))
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
# inner transaction + second part of "outer" transaction were committed # inner transaction + second part of "outer" transaction were committed
with session.begin():
self.assertEqual( self.assertEqual(
[("u2",), ("u3",), ("u4", )], [("u2",), ("u3",), ("u4", )],
session.query( session.query(
@ -1880,9 +1884,9 @@ class LiveFacadeTest(db_test_base._DbTestCase):
go1(context) go1(context)
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
# inner transaction + second part of "outer" transaction were committed # inner transaction + second part of "outer" transaction were committed
with session.begin():
self.assertEqual( self.assertEqual(
[("u1",), ("u3",), ("u4", )], [("u1",), ("u3",), ("u4", )],
session.query( session.query(
@ -1908,9 +1912,9 @@ class LiveFacadeTest(db_test_base._DbTestCase):
session.add(self.User(name="u4")) session.add(self.User(name="u4"))
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
# inner transaction + second part of "outer" transaction were committed # inner transaction + second part of "outer" transaction were committed
with session.begin():
self.assertEqual( self.assertEqual(
[("u1",), ("u3",), ("u4", )], [("u1",), ("u3",), ("u4", )],
session.query( session.query(
@ -1956,9 +1960,9 @@ class LiveFacadeTest(db_test_base._DbTestCase):
go1(context) go1(context)
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
# inner transaction + second part of "outer" transaction were committed # inner transaction + second part of "outer" transaction were committed
with session.begin():
self.assertEqual( self.assertEqual(
[("u2",), ("u3",), ("u4", )], [("u2",), ("u3",), ("u4", )],
session.query( session.query(
@ -1995,7 +1999,8 @@ class LiveFacadeTest(db_test_base._DbTestCase):
# add more state on the "outer" transaction # add more state on the "outer" transaction
connection.execute(self.user_table.insert().values(name="u4")) connection.execute(self.user_table.insert().values(name="u4"))
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
with session.begin():
self.assertEqual( self.assertEqual(
[("u2",), ("u3",), ("u4", )], [("u2",), ("u3",), ("u4", )],
session.query( session.query(
@ -2030,7 +2035,8 @@ class LiveFacadeTest(db_test_base._DbTestCase):
user = session.query(self.User).first() user = session.query(self.User).first()
self.assertEqual("u1_commit", user.name) self.assertEqual("u1_commit", user.name)
session = self.sessionmaker(autocommit=True) session = self.sessionmaker(autocommit=False)
with session.begin():
self.assertEqual( self.assertEqual(
[("u1_commit",)], [("u1_commit",)],
session.query( session.query(

1
oslo_db/tests/sqlalchemy/test_types.py
View File

@ -81,7 +81,6 @@ class JsonTypesTestCase(test_base._DbTestCase):
{'a': 'b'} {'a': 'b'}
] ]
for i, test in enumerate(tested): for i, test in enumerate(tested):
with self.session.begin():
JsonTable(id=i, json=test).save(self.session) JsonTable(id=i, json=test).save(self.session)
obj = self.session.query(JsonTable).filter_by(id=i).one() obj = self.session.query(JsonTable).filter_by(id=i).one()
self.assertEqual(test, obj.json) self.assertEqual(test, obj.json)