diff options
| -rw-r--r-- | oslo_db/sqlalchemy/enginefacade.py | 15 | ||||
| -rw-r--r-- | oslo_db/sqlalchemy/orm.py | 2 | ||||
| -rw-r--r-- | oslo_db/sqlalchemy/test_migrations.py | 12 | ||||
| -rw-r--r-- | oslo_db/sqlalchemy/types.py | 8 | ||||
| -rw-r--r-- | oslo_db/sqlalchemy/utils.py | 22 | ||||
| -rw-r--r-- | oslo_db/tests/fixtures.py | 20 | ||||
| -rw-r--r-- | oslo_db/tests/sqlalchemy/test_enginefacade.py | 130 | ||||
| -rw-r--r-- | oslo_db/tests/sqlalchemy/test_types.py | 3 | ||||
| -rw-r--r-- | releasenotes/source/index.rst | 1 | ||||
| -rw-r--r-- | releasenotes/source/zed.rst | 6 | ||||
| -rw-r--r-- | tox.ini | 2 |
11 files changed, 134 insertions, 87 deletions
diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py index 81341b7..a66528d 100644 --- a/oslo_db/sqlalchemy/enginefacade.py +++ b/oslo_db/sqlalchemy/enginefacade.py @@ -163,7 +163,7 @@ class _TransactionFactory(object): } self._maker_cfg = { 'expire_on_commit': _Default(False), - '__autocommit': True + '__autocommit': False, } self._transaction_ctx_cfg = { 'rollback_reader_sessions': False, @@ -1275,13 +1275,22 @@ class LegacyEngineFacade(object): """ 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): warnings.warn( "EngineFacade is deprecated; please use " "oslo_db.sqlalchemy.enginefacade", warning.OsloDBDeprecationWarning, 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: self._factory = _factory else: @@ -1355,7 +1364,7 @@ class LegacyEngineFacade(object): @classmethod 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. :param conf: oslo.config config instance diff --git a/oslo_db/sqlalchemy/orm.py b/oslo_db/sqlalchemy/orm.py index b1ca00a..a5ec4c4 100644 --- a/oslo_db/sqlalchemy/orm.py +++ b/oslo_db/sqlalchemy/orm.py @@ -57,7 +57,7 @@ class Session(sqlalchemy.orm.session.Session): """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 sqlalchemy.orm.sessionmaker(bind=engine, class_=Session, diff --git a/oslo_db/sqlalchemy/test_migrations.py b/oslo_db/sqlalchemy/test_migrations.py index 74181db..a0b5591 100644 --- a/oslo_db/sqlalchemy/test_migrations.py +++ b/oslo_db/sqlalchemy/test_migrations.py @@ -77,7 +77,8 @@ class WalkVersionsMixin(object, metaclass=abc.ABCMeta): """ - @abc.abstractproperty + @property + @abc.abstractmethod def INIT_VERSION(self): """Initial version of a migration repository. @@ -87,7 +88,8 @@ class WalkVersionsMixin(object, metaclass=abc.ABCMeta): """ pass - @abc.abstractproperty + @property + @abc.abstractmethod def REPOSITORY(self): """Allows basic manipulation with migration repository. @@ -95,7 +97,8 @@ class WalkVersionsMixin(object, metaclass=abc.ABCMeta): """ pass - @abc.abstractproperty + @property + @abc.abstractmethod def migration_api(self): """Provides API for upgrading, downgrading and version manipulations. @@ -103,7 +106,8 @@ class WalkVersionsMixin(object, metaclass=abc.ABCMeta): """ pass - @abc.abstractproperty + @property + @abc.abstractmethod def migrate_engine(self): """Provides engine instance. diff --git a/oslo_db/sqlalchemy/types.py b/oslo_db/sqlalchemy/types.py index b30afce..891c73d 100644 --- a/oslo_db/sqlalchemy/types.py +++ b/oslo_db/sqlalchemy/types.py @@ -70,6 +70,8 @@ class JsonEncodedDict(JsonEncodedType): """ type = dict + cache_ok = True + """This type is safe to cache.""" class JsonEncodedList(JsonEncodedType): @@ -82,6 +84,8 @@ class JsonEncodedList(JsonEncodedType): """ type = list + cache_ok = True + """This type is safe to cache.""" class SoftDeleteInteger(TypeDecorator): @@ -133,9 +137,11 @@ class String(_String): mysql_ndb_type is used to override the String with another data type. mysql_ndb_size is used to adjust the length of the String. - """ + cache_ok = True + """This type is safe to cache.""" + def __init__( self, length, diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 09156f5..83a2cbd 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -39,6 +39,7 @@ from sqlalchemy import Index from sqlalchemy import inspect from sqlalchemy import Integer from sqlalchemy import MetaData +from sqlalchemy import PrimaryKeyConstraint from sqlalchemy.sql.expression import cast from sqlalchemy.sql.expression import literal_column from sqlalchemy.sql import text @@ -608,7 +609,14 @@ def _change_deleted_column_type_to_boolean_sqlite(engine, table_name, # FIXME(stephenfin): We shouldn't be using this private API; # figure out how else to copy an arbitrary column schema - constraints = [constraint._copy() for constraint in table.constraints] + # NOTE(stephenfin): We drop PrimaryKeyConstraint-type constraints since + # these duplicate the 'primary_key=True' attribute on the speicified + # column(s). This technically breaks things when the primary key covers + # multiple columns but that's okay: these are deprecated APIs + constraints = [ + constraint._copy() for constraint in table.constraints + if not isinstance(constraint, PrimaryKeyConstraint) + ] with engine.connect() as conn: meta = table.metadata @@ -738,7 +746,10 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, constraints = [] for constraint in table.constraints: - if not _is_deleted_column_constraint(constraint): + if not ( + _is_deleted_column_constraint(constraint) or + isinstance(constraint, PrimaryKeyConstraint) + ): # FIXME(stephenfin): We shouldn't be using this private API; # figure out how else to copy an arbitrary constraint schema constraints.append(constraint._copy()) @@ -749,7 +760,8 @@ def _change_deleted_column_type_to_id_type_sqlite(engine, table_name, with conn.begin(): new_table = Table( table_name + "__tmp__", meta, - *(columns + constraints)) + *(columns + constraints), + ) new_table.create(conn) indexes = [] @@ -1169,7 +1181,9 @@ def get_non_ndbcluster_tables(connectable, skip_tables=None): params['database'] = connectable.engine.url.database query = text(query_str) - nonndbcluster = connectable.execute(query, **params) + # TODO(stephenfin): What about if this is already a Connection? + with connectable.connect() as conn, conn.begin(): + nonndbcluster = connectable.execute(query, **params) return [i[0] for i in nonndbcluster] diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py index 468dcae..62a88fb 100644 --- a/oslo_db/tests/fixtures.py +++ b/oslo_db/tests/fixtures.py @@ -24,28 +24,28 @@ class WarningsFixture(fixtures.Fixture): self._original_warning_filters = warnings.filters[:] - # Make deprecation warnings only happen once to avoid spamming warnings.simplefilter('once', DeprecationWarning) + # Except things we've deprecated but are still testing until removal + warnings.filterwarnings( - 'error', message='Evaluating non-mapped column expression', - category=sqla_exc.SAWarning) + 'ignore', + category=DeprecationWarning, + module='oslo_db') - # Enable deprecation warnings to capture upcoming SQLAlchemy changes + # Enable generic warnings to ensure we're not doing anything odd warnings.filterwarnings( 'error', - category=sqla_exc.SADeprecationWarning) + category=sqla_exc.SAWarning) - # ...but filter everything out until we get around to fixing them - # FIXME(stephenfin): Remove all of these + # Enable deprecation warnings to capture upcoming SQLAlchemy changes warnings.filterwarnings( - 'once', - message=r'The Session.autocommit parameter is deprecated .*', + 'error', category=sqla_exc.SADeprecationWarning) - # ...plus things that aren't our fault + # ...but filter 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 diff --git a/oslo_db/tests/sqlalchemy/test_enginefacade.py b/oslo_db/tests/sqlalchemy/test_enginefacade.py index a188d01..bdf5104 100644 --- a/oslo_db/tests/sqlalchemy/test_enginefacade.py +++ b/oslo_db/tests/sqlalchemy/test_enginefacade.py @@ -357,11 +357,11 @@ class MockFacadeTest(test_base.BaseTestCase): maker_factories = mock.Mock(side_effect=get_maker) maker_factories( - autocommit=True, engine=engines.writer, + autocommit=False, engine=engines.writer, expire_on_commit=False) if self.slave_uri: maker_factories( - autocommit=True, engine=engines.async_reader, + autocommit=False, engine=engines.async_reader, expire_on_commit=False) yield makers @@ -1692,11 +1692,12 @@ class LiveFacadeTest(db_test_base._DbTestCase): with enginefacade.writer.using(context) as session: session.add(self.User(name="u1")) - session = self.sessionmaker(autocommit=True) - self.assertEqual( - "u1", - session.query(self.User.name).scalar() - ) + session = self.sessionmaker(autocommit=False) + with session.begin(): + self.assertEqual( + "u1", + session.query(self.User.name).scalar() + ) def test_transaction_rollback(self): context = oslo_context.RequestContext() @@ -1712,11 +1713,12 @@ class LiveFacadeTest(db_test_base._DbTestCase): self.assertRaises(MyException, go, context) - session = self.sessionmaker(autocommit=True) - self.assertEqual( - None, - session.query(self.User.name).scalar() - ) + session = self.sessionmaker(autocommit=False) + with session.begin(): + self.assertEqual( + None, + session.query(self.User.name).scalar() + ) @mock.patch.object(Session, 'commit') @mock.patch.object(Session, 'rollback') @@ -1783,11 +1785,12 @@ class LiveFacadeTest(db_test_base._DbTestCase): s2.add(self.User(name="u1")) s2.flush() - session = self.sessionmaker(autocommit=True) - self.assertEqual( - "u1", - session.query(self.User.name).scalar() - ) + session = self.sessionmaker(autocommit=False) + with session.begin(): + self.assertEqual( + "u1", + session.query(self.User.name).scalar() + ) def test_context_deepcopy_on_connection(self): context = oslo_context.RequestContext() @@ -1804,11 +1807,12 @@ class LiveFacadeTest(db_test_base._DbTestCase): self._assert_ctx_connection(ctx2, conn2) - session = self.sessionmaker(autocommit=True) - self.assertEqual( - "u1", - session.query(self.User.name).scalar() - ) + session = self.sessionmaker(autocommit=False) + with session.begin(): + self.assertEqual( + "u1", + session.query(self.User.name).scalar() + ) @db_test_base.backend_specific("postgresql", "mysql") def test_external_session_transaction(self): @@ -1840,14 +1844,14 @@ class LiveFacadeTest(db_test_base._DbTestCase): session.begin() 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 - self.assertEqual( - [("u2",), ("u3",), ("u4", )], - session.query( - self.User.name).order_by(self.User.name).all() - ) + with session.begin(): + self.assertEqual( + [("u2",), ("u3",), ("u4", )], + session.query( + self.User.name).order_by(self.User.name).all() + ) def test_savepoint_transaction_decorator(self): context = oslo_context.RequestContext() @@ -1880,14 +1884,14 @@ class LiveFacadeTest(db_test_base._DbTestCase): go1(context) - session = self.sessionmaker(autocommit=True) - + session = self.sessionmaker(autocommit=False) # inner transaction + second part of "outer" transaction were committed - self.assertEqual( - [("u1",), ("u3",), ("u4", )], - session.query( - self.User.name).order_by(self.User.name).all() - ) + with session.begin(): + self.assertEqual( + [("u1",), ("u3",), ("u4", )], + session.query( + self.User.name).order_by(self.User.name).all() + ) def test_savepoint_transaction(self): context = oslo_context.RequestContext() @@ -1908,14 +1912,14 @@ class LiveFacadeTest(db_test_base._DbTestCase): 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 - self.assertEqual( - [("u1",), ("u3",), ("u4", )], - session.query( - self.User.name).order_by(self.User.name).all() - ) + with session.begin(): + self.assertEqual( + [("u1",), ("u3",), ("u4", )], + session.query( + self.User.name).order_by(self.User.name).all() + ) @db_test_base.backend_specific("postgresql", "mysql") def test_external_session_transaction_decorator(self): @@ -1956,14 +1960,14 @@ class LiveFacadeTest(db_test_base._DbTestCase): go1(context) - session = self.sessionmaker(autocommit=True) - + session = self.sessionmaker(autocommit=False) # inner transaction + second part of "outer" transaction were committed - self.assertEqual( - [("u2",), ("u3",), ("u4", )], - session.query( - self.User.name).order_by(self.User.name).all() - ) + with session.begin(): + self.assertEqual( + [("u2",), ("u3",), ("u4", )], + session.query( + self.User.name).order_by(self.User.name).all() + ) @db_test_base.backend_specific("postgresql", "mysql") def test_external_connection_transaction(self): @@ -1995,12 +1999,13 @@ class LiveFacadeTest(db_test_base._DbTestCase): # add more state on the "outer" transaction connection.execute(self.user_table.insert().values(name="u4")) - session = self.sessionmaker(autocommit=True) - self.assertEqual( - [("u2",), ("u3",), ("u4", )], - session.query( - self.User.name).order_by(self.User.name).all() - ) + session = self.sessionmaker(autocommit=False) + with session.begin(): + self.assertEqual( + [("u2",), ("u3",), ("u4", )], + session.query( + self.User.name).order_by(self.User.name).all() + ) @db_test_base.backend_specific("postgresql", "mysql") def test_external_writer_in_reader(self): @@ -2030,12 +2035,13 @@ class LiveFacadeTest(db_test_base._DbTestCase): user = session.query(self.User).first() self.assertEqual("u1_commit", user.name) - session = self.sessionmaker(autocommit=True) - self.assertEqual( - [("u1_commit",)], - session.query( - self.User.name).order_by(self.User.name).all() - ) + session = self.sessionmaker(autocommit=False) + with session.begin(): + self.assertEqual( + [("u1_commit",)], + session.query( + self.User.name).order_by(self.User.name).all() + ) def test_replace_scope(self): # "timeout" is an argument accepted by diff --git a/oslo_db/tests/sqlalchemy/test_types.py b/oslo_db/tests/sqlalchemy/test_types.py index 4b43665..cf0c42a 100644 --- a/oslo_db/tests/sqlalchemy/test_types.py +++ b/oslo_db/tests/sqlalchemy/test_types.py @@ -81,8 +81,7 @@ class JsonTypesTestCase(test_base._DbTestCase): {'a': 'b'} ] 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() self.assertEqual(test, obj.json) diff --git a/releasenotes/source/index.rst b/releasenotes/source/index.rst index 6d3ce9e..a947111 100644 --- a/releasenotes/source/index.rst +++ b/releasenotes/source/index.rst @@ -6,6 +6,7 @@ :maxdepth: 1 unreleased + zed yoga xena wallaby diff --git a/releasenotes/source/zed.rst b/releasenotes/source/zed.rst new file mode 100644 index 0000000..9608c05 --- /dev/null +++ b/releasenotes/source/zed.rst @@ -0,0 +1,6 @@ +======================== +Zed Series Release Notes +======================== + +.. release-notes:: + :branch: stable/zed @@ -10,6 +10,8 @@ allowlist_externals = passenv = OS_TEST_DBAPI_ADMIN_CONNECTION setenv = + OS_STDOUT_CAPTURE=true + OS_STDERR_CAPTURE=true BASECOMMAND=stestr run {postgresql,all}: PIFPAF_POSTGRESQL=pifpaf -g OS_TEST_DBAPI_ADMIN_CONNECTION run postgresql -- {mysql,all}: PIFPAF_MYSQL=pifpaf -g OS_TEST_DBAPI_ADMIN_CONNECTION run mysql -- |