diff --git a/doc/source/usage.rst b/doc/source/usage.rst index 11c8c2a3..cc4f2fdb 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -76,6 +76,12 @@ positional argument: results = some_reader_api_function(context) some_writer_api_function(context, 5, 10) +.. note:: The ``context.session`` and ``context.connection`` attributes + must be accessed within the scope of an appropriate writer/reader block + (either the decorator or contextmanager approach). An AttributeError is + raised otherwise. + + The scope of transaction and connectivity for both approaches is managed transparently. The configuration for the connection comes from the standard :obj:`oslo_config.cfg.CONF` collection. Additional configurations can be diff --git a/oslo_db/exception.py b/oslo_db/exception.py index c1620e1c..2b2df307 100644 --- a/oslo_db/exception.py +++ b/oslo_db/exception.py @@ -208,12 +208,12 @@ class RetryRequest(Exception): class NoEngineContextEstablished(AttributeError): - """Error raised for non-present enginefacade attribute access. + """Error raised for enginefacade attribute access with no context. This applies to the ``session`` and ``connection`` attributes of a user-defined context and/or RequestContext object, when they - are accessed outside of the scope of an enginefacade decorator + are accessed *outside* of the scope of an enginefacade decorator or context manager. The exception is a subclass of AttributeError so that @@ -224,6 +224,23 @@ class NoEngineContextEstablished(AttributeError): """ +class ContextNotRequestedError(AttributeError): + """Error raised when requesting a not-setup enginefacade attribute. + + This applies to the ``session`` and ``connection`` attributes + of a user-defined context and/or RequestContext object, when they + are accessed *within* the scope of an enginefacade decorator + or context manager, but the context has not requested that + attribute (e.g. like "with enginefacade.connection.using(context)" + and "context.session" is requested). + + """ + + +class CantStartEngineError(Exception): + """Error raised when the enginefacade cannot start up correctly.""" + + class NotSupportedWarning(Warning): """Warn that an argument or call that was passed is not supported. diff --git a/oslo_db/sqlalchemy/enginefacade.py b/oslo_db/sqlalchemy/enginefacade.py index 4497a50b..cf7f1490 100644 --- a/oslo_db/sqlalchemy/enginefacade.py +++ b/oslo_db/sqlalchemy/enginefacade.py @@ -314,7 +314,6 @@ class _TransactionFactory(object): # for the lock. if self._started: return - self._started = True if conf is False: conf = cfg.CONF @@ -349,8 +348,16 @@ class _TransactionFactory(object): self.synchronous_reader = self._facade_cfg['synchronous_reader'] + # set up _started last, so that in case of exceptions + # we try the whole thing again and report errors + # correctly + self._started = True + def _setup_for_connection( self, sql_connection, engine_kwargs, maker_kwargs): + if sql_connection is None: + raise exception.CantStartEngineError( + "No sql_connection parameter is established") engine = engines.create_engine( sql_connection=sql_connection, **engine_kwargs) sessionmaker = orm.get_maker(engine=engine, **maker_kwargs) @@ -736,7 +743,13 @@ def _context_descriptor(attr=None): % (context, attr) ) else: - return getter(transaction_context) + result = getter(transaction_context) + if result is None: + raise exception.ContextNotRequestedError( + "The '%s' context attribute was requested but " + "it has not been established for this context." % attr + ) + return result return property(_property_for_context) diff --git a/oslo_db/tests/sqlalchemy/test_enginefacade.py b/oslo_db/tests/sqlalchemy/test_enginefacade.py index fe8030c6..6893a933 100644 --- a/oslo_db/tests/sqlalchemy/test_enginefacade.py +++ b/oslo_db/tests/sqlalchemy/test_enginefacade.py @@ -770,6 +770,36 @@ class MockFacadeTest(oslo_test_base.BaseTestCase): conn.execute("test1") conn.execute("test2") + def test_session_context_notrequested_exception(self): + context = oslo_context.RequestContext() + + with enginefacade.reader.connection.using(context): + exc = self.assertRaises( + exception.ContextNotRequestedError, + getattr, context, 'session' + ) + + self.assertRegexpMatches( + exc.args[0], + "The 'session' context attribute was requested but it has " + "not been established for this context." + ) + + def test_connection_context_notrequested_exception(self): + context = oslo_context.RequestContext() + + with enginefacade.reader.using(context): + exc = self.assertRaises( + exception.ContextNotRequestedError, + getattr, context, 'connection' + ) + + self.assertRegexpMatches( + exc.args[0], + "The 'connection' context attribute was requested but it has " + "not been established for this context." + ) + def test_session_context_exception(self): context = oslo_context.RequestContext() exc = self.assertRaises( @@ -1052,19 +1082,28 @@ class ThreadingTest(test_base.DbTestCase): @enginefacade.writer.connection def go_one(context): - self.assertIsNone(context.session) + self.assertRaises( + exception.ContextNotRequestedError, + getattr, context, "session" + ) self.assertIsNotNone(context.connection) self.ident = 2 go_two(context) self.ident = 1 - self.assertIsNone(context.session) + self.assertRaises( + exception.ContextNotRequestedError, + getattr, context, "session" + ) self.assertIsNotNone(context.connection) @enginefacade.reader def go_two(context): - self.assertIsNone(context.connection) + self.assertRaises( + exception.ContextNotRequestedError, + getattr, context, "connection" + ) self.assertIsNotNone(context.session) context = oslo_context.RequestContext() @@ -1652,5 +1691,19 @@ class ConfigOptionsTest(oslo_test_base.BaseTestCase): str(w[-1].message) ) + def test_no_engine(self): + factory = enginefacade._TransactionFactory() + + self.assertRaises( + exception.CantStartEngineError, + factory._create_session, enginefacade._WRITER + ) + + self.assertRaises( + exception.CantStartEngineError, + factory._create_session, enginefacade._WRITER + ) + + # TODO(zzzeek): test configuration options, e.g. like # test_sqlalchemy->test_creation_from_config