From 80d7448e80ae4b9883ef0b9d24be5dbd09ee664c Mon Sep 17 00:00:00 2001 From: Sergey Nikitin Date: Thu, 5 Nov 2015 21:05:43 +0300 Subject: [PATCH] Use EngineFacade from oslo_db.enginefacade 'get_session' and 'get_api_session' methods are still needed for backward compatibility. Implements: blueprint new-oslodb-enginefacade Closes-Bug: #1502104 Co-Authored-By: Pavel Kholkin Change-Id: I8ceb9f939470f744f0d616d7db065a4d5d80202b --- nova/config.py | 6 ++- nova/context.py | 2 + nova/db/sqlalchemy/api.py | 52 ++++++++----------- nova/tests/fixtures.py | 7 +++ nova/tests/unit/conf_fixture.py | 2 +- nova/tests/unit/db/test_db_api.py | 83 ++++++++++++++++++++++++++----- 6 files changed, 107 insertions(+), 45 deletions(-) diff --git a/nova/config.py b/nova/config.py index 4e0950a12639..b3c9902efba2 100644 --- a/nova/config.py +++ b/nova/config.py @@ -19,6 +19,7 @@ from oslo_config import cfg from oslo_db import options from oslo_log import log +from nova.db.sqlalchemy import api as sqlalchemy_api from nova import debugger from nova import paths from nova import rpc @@ -46,7 +47,7 @@ _DEFAULT_LOGGING_CONTEXT_FORMAT = ('%(asctime)s.%(msecs)03d %(process)d ' '%(message)s') -def parse_args(argv, default_config_files=None): +def parse_args(argv, default_config_files=None, configure_db=True): log.set_defaults(_DEFAULT_LOGGING_CONTEXT_FORMAT, _DEFAULT_LOG_LEVELS) log.register_options(CONF) options.set_defaults(CONF, connection=_DEFAULT_SQL_CONNECTION, @@ -58,3 +59,6 @@ def parse_args(argv, default_config_files=None): version=version.version_string(), default_config_files=default_config_files) rpc.init(CONF) + + if configure_db: + sqlalchemy_api.configure(CONF) diff --git a/nova/context.py b/nova/context.py index e73f4e8d39bc..4537eedc6266 100644 --- a/nova/context.py +++ b/nova/context.py @@ -22,6 +22,7 @@ import copy from keystoneclient import auth from keystoneclient import service_catalog 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 @@ -60,6 +61,7 @@ class _ContextAuthPlugin(auth.BaseAuthPlugin): region_name=region_name) +@enginefacade.transaction_context_provider class RequestContext(context.RequestContext): """Security context and request information. diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 38b0a276fce1..425e7080d687 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -22,14 +22,13 @@ import copy import datetime import functools import sys -import threading import uuid from oslo_config import cfg from oslo_db import api as oslo_db_api from oslo_db import exception as db_exc from oslo_db import options as oslo_db_options -from oslo_db.sqlalchemy import session as db_session +from oslo_db.sqlalchemy import enginefacade from oslo_db.sqlalchemy import update_match from oslo_db.sqlalchemy import utils as sqlalchemyutils from oslo_log import log as logging @@ -130,20 +129,16 @@ CONF.import_opt('until_refresh', 'nova.quota') LOG = logging.getLogger(__name__) -_ENGINE_FACADE = {'main': None, 'api': None} -_MAIN_FACADE = 'main' -_API_FACADE = 'api' -_LOCK = threading.Lock() +main_context_manager = enginefacade.transaction_context() +api_context_manager = enginefacade.transaction_context() -def _create_facade(conf_group): - - # NOTE(dheeraj): This fragment is copied from oslo.db - return db_session.EngineFacade( - sql_connection=conf_group.connection, +def _get_db_conf(conf_group): + kw = dict( + connection=conf_group.connection, slave_connection=conf_group.slave_connection, sqlite_fk=False, - autocommit=True, + __autocommit=True, expire_on_commit=False, mysql_sql_mode=conf_group.mysql_sql_mode, idle_timeout=conf_group.idle_timeout, @@ -155,39 +150,31 @@ def _create_facade(conf_group): connection_trace=conf_group.connection_trace, max_retries=conf_group.max_retries, retry_interval=conf_group.retry_interval) + return kw -def _create_facade_lazily(facade, conf_group): - global _LOCK, _ENGINE_FACADE - if _ENGINE_FACADE[facade] is None: - with _LOCK: - if _ENGINE_FACADE[facade] is None: - _ENGINE_FACADE[facade] = _create_facade(conf_group) - return _ENGINE_FACADE[facade] +def configure(conf): + main_context_manager.configure(**_get_db_conf(conf.database)) + api_context_manager.configure(**_get_db_conf(conf.api_database)) def get_engine(use_slave=False): - conf_group = CONF.database - facade = _create_facade_lazily(_MAIN_FACADE, conf_group) - return facade.get_engine(use_slave=use_slave) + return main_context_manager._factory.get_legacy_facade().get_engine( + use_slave=use_slave) def get_api_engine(): - conf_group = CONF.api_database - facade = _create_facade_lazily(_API_FACADE, conf_group) - return facade.get_engine() + return api_context_manager._factory.get_legacy_facade().get_engine() def get_session(use_slave=False, **kwargs): - conf_group = CONF.database - facade = _create_facade_lazily(_MAIN_FACADE, conf_group) - return facade.get_session(use_slave=use_slave, **kwargs) + return main_context_manager._factory.get_legacy_facade().get_session( + use_slave=use_slave, **kwargs) def get_api_session(**kwargs): - conf_group = CONF.api_database - facade = _create_facade_lazily(_API_FACADE, conf_group) - return facade.get_session(**kwargs) + return api_context_manager._factory.get_legacy_facade().get_session( + **kwargs) _SHADOW_TABLE_PREFIX = 'shadow_' @@ -270,6 +257,9 @@ def model_query(context, model, 'allow_none', restriction includes project_id = None. """ + if hasattr(context, 'session'): + session = context.session + if session is None: if CONF.database.slave_connection == '': use_slave = False diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index 35c4692123bb..33adee045fa4 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -39,6 +39,7 @@ _TRUE_VALUES = ('True', 'true', '1', 'yes') CONF = cfg.CONF DB_SCHEMA = {'main': "", 'api': ""} +SESSION_CONFIGURED = False class ServiceFixture(fixtures.Fixture): @@ -195,6 +196,12 @@ class Timeout(fixtures.Fixture): class Database(fixtures.Fixture): def __init__(self, database='main'): super(Database, self).__init__() + # NOTE(pkholkin): oslo_db.enginefacade is configured in tests the same + # way as it is done for any other service that uses db + global SESSION_CONFIGURED + if not SESSION_CONFIGURED: + session.configure(CONF) + SESSION_CONFIGURED = True self.database = database if database == 'main': self.get_engine = session.get_engine diff --git a/nova/tests/unit/conf_fixture.py b/nova/tests/unit/conf_fixture.py index 3fabf5e2e4e0..3cbd828da3fc 100644 --- a/nova/tests/unit/conf_fixture.py +++ b/nova/tests/unit/conf_fixture.py @@ -56,7 +56,7 @@ class ConfFixture(config_fixture.Config): self.conf.set_default('use_ipv6', True) self.conf.set_default('vlan_interface', 'eth0') self.conf.set_default('auth_strategy', 'noauth2') - config.parse_args([], default_config_files=[]) + config.parse_args([], default_config_files=[], configure_db=False) self.conf.set_default('connection', "sqlite://", group='database') self.conf.set_default('connection', "sqlite://", group='api_database') self.conf.set_default('sqlite_synchronous', False, group='database') diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 0f154155aaa0..9a833f34a358 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -501,6 +501,65 @@ class ModelQueryTestCase(DbTestCase): session=mock.MagicMock()) self.assertFalse(mock_get_session.called) + @mock.patch.object(sqlalchemy_api, 'get_session') + @mock.patch.object(sqlalchemyutils, 'model_query') + def test_model_query_use_context_session(self, mock_model_query, + mock_get_session): + @sqlalchemy_api.main_context_manager.reader + def fake_method(context): + session = context.session + sqlalchemy_api.model_query(context, models.Instance) + return session + + session = fake_method(self.context) + self.assertFalse(mock_get_session.called) + mock_model_query.assert_called_once_with(models.Instance, session, + None, deleted=False) + + +class EngineFacadeTestCase(DbTestCase): + @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.Instance) + return session + + parent_session, child_session = fake_parent_method(self.context) + self.assertFalse(mock_get_session.called) + 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.Instance) + return session + + parent_session, child_session = fake_parent_method(self.context) + self.assertFalse(mock_get_session.called) + self.assertEqual(parent_session, child_session) + class AggregateDBApiTestCase(test.TestCase): def setUp(self): @@ -968,44 +1027,44 @@ class SqlAlchemyDbApiNoDbTestCase(test.NoDBTestCase): op = sqlalchemy_api._get_regexp_op_for_connection('notdb:///') self.assertEqual('LIKE', op) - @mock.patch.object(sqlalchemy_api, '_create_facade_lazily') + @mock.patch.object(sqlalchemy_api.main_context_manager._factory, + 'get_legacy_facade') def test_get_engine(self, mock_create_facade): mock_facade = mock.MagicMock() mock_create_facade.return_value = mock_facade sqlalchemy_api.get_engine() - mock_create_facade.assert_called_once_with(sqlalchemy_api._MAIN_FACADE, - CONF.database) + mock_create_facade.assert_called_once_with() mock_facade.get_engine.assert_called_once_with(use_slave=False) - @mock.patch.object(sqlalchemy_api, '_create_facade_lazily') + @mock.patch.object(sqlalchemy_api.api_context_manager._factory, + 'get_legacy_facade') def test_get_api_engine(self, mock_create_facade): mock_facade = mock.MagicMock() mock_create_facade.return_value = mock_facade sqlalchemy_api.get_api_engine() - mock_create_facade.assert_called_once_with(sqlalchemy_api._API_FACADE, - CONF.api_database) + mock_create_facade.assert_called_once_with() mock_facade.get_engine.assert_called_once_with() - @mock.patch.object(sqlalchemy_api, '_create_facade_lazily') + @mock.patch.object(sqlalchemy_api.main_context_manager._factory, + 'get_legacy_facade') def test_get_session(self, mock_create_facade): mock_facade = mock.MagicMock() mock_create_facade.return_value = mock_facade sqlalchemy_api.get_session() - mock_create_facade.assert_called_once_with(sqlalchemy_api._MAIN_FACADE, - CONF.database) + mock_create_facade.assert_called_once_with() mock_facade.get_session.assert_called_once_with(use_slave=False) - @mock.patch.object(sqlalchemy_api, '_create_facade_lazily') + @mock.patch.object(sqlalchemy_api.api_context_manager._factory, + 'get_legacy_facade') def test_get_api_session(self, mock_create_facade): mock_facade = mock.MagicMock() mock_create_facade.return_value = mock_facade sqlalchemy_api.get_api_session() - mock_create_facade.assert_called_once_with(sqlalchemy_api._API_FACADE, - CONF.api_database) + mock_create_facade.assert_called_once_with() mock_facade.get_session.assert_called_once_with() @mock.patch.object(sqlalchemy_api, '_instance_get_by_uuid')