Merge "Use oslo_db.sqlalchemy.enginefacade"

cinder/context.py

@@ -21,6 +21,7 @@ import copy
 
 from oslo_config import cfg
 from oslo_context import context
+from oslo_db.sqlalchemy import enginefacade
 from oslo_log import log as logging
 from oslo_utils import timeutils
 import six
@@ -45,6 +46,7 @@ CONF.register_opts(context_opts)
 LOG = logging.getLogger(__name__)
 
 
+@enginefacade.transaction_context_provider
 class RequestContext(context.RequestContext):
     """Security context and request information.
 

cinder/db/sqlalchemy/api.py

@@ -25,7 +25,6 @@ import functools
 import itertools
 import re
 import sys
-import threading
 import time
 import uuid
 
@@ -33,7 +32,6 @@ from oslo_config import cfg
 from oslo_db import exception as db_exc
 from oslo_db import options
 from oslo_db.sqlalchemy import enginefacade
-from oslo_db.sqlalchemy import session as db_session
 from oslo_log import log as logging
 from oslo_utils import importutils
 from oslo_utils import timeutils
@@ -69,42 +67,30 @@ LOG = logging.getLogger(__name__)
 
 options.set_defaults(CONF, connection='sqlite:///$state_path/cinder.sqlite')
 
-_LOCK = threading.Lock()
+main_context_manager = enginefacade.transaction_context()
-_FACADE = None
 
 
-def _create_facade_lazily():
+def configure(conf):
-    global _LOCK
+    main_context_manager.configure(**dict(conf.database))
-    with _LOCK:
+    # NOTE(geguileo): To avoid a cyclical dependency we import the
-        global _FACADE
+    # group here.  Dependency cycle is objects.base requires db.api,
-        if _FACADE is None:
+    # which requires db.sqlalchemy.api, which requires service which
-            _FACADE = db_session.EngineFacade(
+    # requires objects.base
-                CONF.database.connection,
+    CONF.import_group("profiler", "cinder.service")
-                **dict(CONF.database)
+    if CONF.profiler.enabled:
-            )
+        if CONF.profiler.trace_sqlalchemy:
-
+            lambda eng: osprofiler_sqlalchemy.add_tracing(sqlalchemy,
-            # NOTE(geguileo): To avoid a cyclical dependency we import the
+                                                          eng, "db")
-            # group here.  Dependency cycle is objects.base requires db.api,
-            # which requires db.sqlalchemy.api, which requires service which
-            # requires objects.base
-            CONF.import_group("profiler", "cinder.service")
-            if CONF.profiler.enabled:
-                if CONF.profiler.trace_sqlalchemy:
-                    osprofiler_sqlalchemy.add_tracing(sqlalchemy,
-                                                      _FACADE.get_engine(),
-                                                      "db")
-
-        return _FACADE
 
 
-def get_engine():
+def get_engine(use_slave=False):
-    facade = _create_facade_lazily()
+    return main_context_manager._factory.get_legacy_facade().get_engine(
-    return facade.get_engine()
+        use_slave=use_slave)
 
 
-def get_session(**kwargs):
+def get_session(use_slave=False, **kwargs):
-    facade = _create_facade_lazily()
+    return main_context_manager._factory.get_legacy_facade().get_session(
-    return facade.get_session(**kwargs)
+        use_slave=use_slave, **kwargs)
 
 
 def dispose_engine():
@@ -275,11 +261,12 @@ def handle_db_data_error(f):
 def model_query(context, model, *args, **kwargs):
     """Query helper that accounts for context's `read_deleted` field.
 
-    :param context: context to query under
+    :param context:      context to query under
-    :param session: if present, the session to use
+    :param model:        Model to query. Must be a subclass of ModelBase.
+    :param args:         Arguments to query. If None - model is used.
     :param read_deleted: if present, overrides context's read_deleted field.
     :param project_only: if present and context is user-type, then restrict
-            query to match the context's project_id.
+                         query to match the context's project_id.
     """
     session = kwargs.get('session') or get_session()
     read_deleted = kwargs.get('read_deleted') or context.read_deleted

cinder/test.py

@@ -57,6 +57,7 @@ from cinder.volume import utils
 CONF = cfg.CONF
 
 _DB_CACHE = None
+SESSION_CONFIGURED = False
 
 
 class TestingException(Exception):
@@ -66,6 +67,12 @@ class TestingException(Exception):
 class Database(fixtures.Fixture):
 
     def __init__(self, db_api, db_migrate, sql_connection):
+        # NOTE(lhx_): oslo_db.enginefacade is configured in tests the same
+        # way as it's done for any other services that uses the db
+        global SESSION_CONFIGURED
+        if not SESSION_CONFIGURED:
+            sqla_api.configure(CONF)
+            SESSION_CONFIGURED = True
         self.sql_connection = sql_connection
 
         # Suppress logging for test runs

cinder/tests/unit/objects/test_base.py

@@ -719,15 +719,6 @@ class TestCinderObjectConditionalUpdate(test.TestCase):
                           {objects.Backup.model.status: 'available',
                            objects.Snapshot.model.status: 'available'})
 
-    def test_conditional_update_not_multitable(self):
-        volume = self._create_volume()
-        with mock.patch('cinder.db.sqlalchemy.api._create_facade_lazily') as m:
-            res = volume.conditional_update(
-                {objects.Volume.model.status: 'deleting',
-                 objects.Volume.model.size: 12}, reflect_changes=False)
-            self.assertTrue(res)
-            self.assertTrue(m.called)
-
     @ddt.data(('available', 'error', None),
               ('error', 'rolling_back', [{'fake_filter': 'faked'}]))
     @ddt.unpack

cinder/tests/unit/test_db_api.py

@@ -3133,6 +3133,56 @@ class DBAPIGenericTestCase(BaseTest):
         self.assertTrue(res, msg="Admin cannot find the Snapshot")
 
 
+class EngineFacadeTestCase(BaseTest):
+
+    """Tests for message operations"""
+    def setUp(self):
+        super(EngineFacadeTestCase, self).setUp()
+        self.user_id = fake.USER_ID
+        self.project_id = fake.PROJECT_ID
+        self.context = context.RequestContext(self.user_id, self.project_id)
+
+    @mock.patch.object(sqlalchemy_api, 'get_session')
+    def test_use_single_context_session_writer(self, mock_get_session):
+        # Checks that session in context would not be overwritten by
+        # annotation @sqlalchemy_api.main_context_manager.writer if annotation
+        # is used twice.
+
+        @sqlalchemy_api.main_context_manager.writer
+        def fake_parent_method(context):
+            session = context.session
+            return fake_child_method(context), session
+
+        @sqlalchemy_api.main_context_manager.writer
+        def fake_child_method(context):
+            session = context.session
+            sqlalchemy_api.model_query(context, models.Volume)
+            return session
+
+        parent_session, child_session = fake_parent_method(self.context)
+        self.assertEqual(parent_session, child_session)
+
+    @mock.patch.object(sqlalchemy_api, 'get_session')
+    def test_use_single_context_session_reader(self, mock_get_session):
+        # Checks that session in context would not be overwritten by
+        # annotation @sqlalchemy_api.main_context_manager.reader if annotation
+        # is used twice.
+
+        @sqlalchemy_api.main_context_manager.reader
+        def fake_parent_method(context):
+            session = context.session
+            return fake_child_method(context), session
+
+        @sqlalchemy_api.main_context_manager.reader
+        def fake_child_method(context):
+            session = context.session
+            sqlalchemy_api.model_query(context, models.Volume)
+            return session
+
+        parent_session, child_session = fake_parent_method(self.context)
+        self.assertEqual(parent_session, child_session)
+
+
 @ddt.ddt
 class DBAPIBackendTestCase(BaseTest):
     @ddt.data((True, True), (True, False), (False, True), (False, False))