Skip to content

Commit

Permalink
Enhanced fixtures for enginefacade-based provisioning
Browse files Browse the repository at this point in the history
The original idea of enginefacade was that applications
would call upon the global _TransactionContextManager
given in oslo_db.sqlalchemy.enginefacade.  However, as it
turns out, virtually no Openstack projects seem to be
using that technique, and instead, everyone is creating
their own ad-hoc _TransactionContextManager objects
and usually establishing it as a module-level global.
Nova has two of them.

Additionally, projects add configuration to these
enginefacades (which IS part of the original idea), and
this configuration in some cases is necessary to be present
for tests that run as well, a key example being the
sqlite_fks flag.   The original DbFixture integration
provided no way of reusing this configuration.

Finally, projects very much tend to use custom fixtures
in order to define their database communication.
Test classes themselves don't really make use of
oslo_db's DbTestCase anymore.

This patch introduces a modernized fixture system
which, in conjunction with the recent provisioning
patch, addresses these use cases.   Applications will typically
create their own subclasses of these fixtures up front
to suit the various testing cases they have, including
SQLite fixed, SQLite ad-hoc, and opportunistic.

In order to accommodate the fixture-based flow
along with the use of testresources for opportunistic
database provisioning, a mixin class OpportunisticDbTestMixin
is still needed when a test needs to use "opportunistic"
testing in order to provide the .resources attribute.
The calculation of .resources is moved into the fixture
system, but because this attribute is consulted before
setUp(), the "opportunistic" fixture must be created
early and stored.

Closes-Bug: #1548960

Change-Id: I0163e637ffef6d45d2573ebe29b5438911d01fce
  • Loading branch information
zzzeek authored and Roman Podoliaka committed Nov 4, 2016
1 parent 3a6d159 commit 2ad571c
Show file tree
Hide file tree
Showing 19 changed files with 916 additions and 39 deletions.
70 changes: 70 additions & 0 deletions oslo_db/sqlalchemy/enginefacade.py
Expand Up @@ -262,6 +262,46 @@ def get_legacy_facade(self):

return self._legacy_facade

def get_writer_engine(self):
"""Return the writer engine for this factory.
Implies start.
"""
if not self._started:
self._start()
return self._writer_engine

def get_reader_engine(self):
"""Return the reader engine for this factory.
Implies start.
"""
if not self._started:
self._start()
return self._reader_engine

def get_writer_maker(self):
"""Return the writer sessionmaker for this factory.
Implies start.
"""
if not self._started:
self._start()
return self._writer_maker

def get_reader_maker(self):
"""Return the reader sessionmaker for this factory.
Implies start.
"""
if not self._started:
self._start()
return self._reader_maker

def _create_connection(self, mode):
if not self._started:
self._start()
Expand Down Expand Up @@ -666,6 +706,36 @@ def get_legacy_facade(self):

return self._factory.get_legacy_facade()

def get_engine(self):
"""Return the Engine in use.
This will be based on the state being WRITER or READER.
This implies a start operation.
"""
if self._mode is _WRITER:
return self._factory.get_writer_engine()
elif self._mode is _READER:
return self._factory.get_reader_engine()
else:
raise ValueError("mode should be WRITER or READER")

def get_sessionmaker(self):
"""Return the sessionmaker in use.
This will be based on the state being WRITER or READER.
This implies a start operation.
"""
if self._mode is _WRITER:
return self._factory.get_writer_maker()
elif self._mode is _READER:
return self._factory.get_reader_maker()
else:
raise ValueError("mode should be WRITER or READER")

def dispose_pool(self):
"""Call engine.pool.dispose() on underlying Engine objects."""
self._factory.dispose_pool()
Expand Down
63 changes: 49 additions & 14 deletions oslo_db/sqlalchemy/provision.py
Expand Up @@ -76,14 +76,24 @@ class Schema(object):


class BackendResource(testresources.TestResourceManager):
def __init__(self, database_type):
def __init__(self, database_type, ad_hoc_url=None):
super(BackendResource, self).__init__()
self.database_type = database_type
self.backend = Backend.backend_for_database_type(self.database_type)
self.ad_hoc_url = ad_hoc_url
if ad_hoc_url is None:
self.backend = Backend.backend_for_database_type(
self.database_type)
else:
self.backend = Backend(self.database_type, ad_hoc_url)
self.backend._verify()

def make(self, dependency_resources):
return self.backend

def clean(self, resource):
self.backend._dispose()

def isDirty(self):
return False

Expand All @@ -100,9 +110,11 @@ class DatabaseResource(testresources.TestResourceManager):
"""

def __init__(self, database_type, _enginefacade=None):
def __init__(self, database_type, _enginefacade=None,
provision_new_database=False, ad_hoc_url=None):
super(DatabaseResource, self).__init__()
self.database_type = database_type
self.provision_new_database = provision_new_database

# NOTE(zzzeek) the _enginefacade is an optional argument
# here in order to accomodate Neutron's current direct use
Expand All @@ -114,38 +126,42 @@ def __init__(self, database_type, _enginefacade=None):
else:
self._enginefacade = enginefacade._context_manager
self.resources = [
('backend', BackendResource(database_type))
('backend', BackendResource(database_type, ad_hoc_url))
]

def make(self, dependency_resources):
backend = dependency_resources['backend']
_enginefacade = self._enginefacade.make_new_manager()

db_token = _random_ident()
url = backend.provisioned_database_url(db_token)
if self.provision_new_database:
db_token = _random_ident()
url = backend.provisioned_database_url(db_token)
LOG.info(
"CREATE BACKEND %s TOKEN %s", backend.engine.url, db_token)
backend.create_named_database(db_token, conditional=True)
else:
db_token = None
url = backend.url

_enginefacade.configure(
logging_name="%s@%s" % (self.database_type, db_token))

LOG.info(
"CREATE BACKEND %s TOKEN %s", backend.engine.url, db_token)
backend.create_named_database(db_token, conditional=True)

_enginefacade._factory._start(connection=url)
engine = _enginefacade._factory._writer_engine
return ProvisionedDatabase(backend, _enginefacade, engine, db_token)

def clean(self, resource):
resource.engine.dispose()
LOG.info(
"DROP BACKEND %s TOKEN %s",
resource.backend.engine, resource.db_token)
resource.backend.drop_named_database(resource.db_token)
if self.provision_new_database:
LOG.info(
"DROP BACKEND %s TOKEN %s",
resource.backend.engine, resource.db_token)
resource.backend.drop_named_database(resource.db_token)

def isDirty(self):
return False


@debtcollector.removals.removed_class("TransactionResource")
class TransactionResource(testresources.TestResourceManager):

def __init__(self, database_resource, schema_resource):
Expand Down Expand Up @@ -299,6 +315,10 @@ def _ensure_backend_available(cls, url):
conn.close()
return eng

def _dispose(self):
"""Dispose main resources of this backend."""
self.impl.dispose(self.engine)

def create_named_database(self, ident, conditional=False):
"""Create a database with the given name."""

Expand Down Expand Up @@ -400,6 +420,10 @@ class BackendImpl(object):

supports_drop_fk = True

def dispose(self, engine):
LOG.info("DISPOSE ENGINE %s", engine)
engine.dispose()

@classmethod
def all_impls(cls):
"""Return an iterator of all possible BackendImpl objects.
Expand Down Expand Up @@ -567,6 +591,17 @@ class SQLiteBackendImpl(BackendImpl):

supports_drop_fk = False

def dispose(self, engine):
LOG.info("DISPOSE ENGINE %s", engine)
engine.dispose()
url = engine.url
self._drop_url_file(url, True)

def _drop_url_file(self, url, conditional):
filename = url.database
if filename and (not conditional or os.access(filename, os.F_OK)):
os.remove(filename)

def create_opportunistic_driver_url(self):
return "sqlite://"

Expand Down
8 changes: 8 additions & 0 deletions oslo_db/sqlalchemy/test_base.py
Expand Up @@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.

import debtcollector
import fixtures
import testresources
import testscenarios
Expand All @@ -35,6 +36,7 @@
from oslo_db.sqlalchemy import session


@debtcollector.removals.removed_class("DbFixture")
class DbFixture(fixtures.Fixture):
"""Basic database fixture.
Expand Down Expand Up @@ -90,6 +92,7 @@ def setUp(self):
self.addCleanup(self.test.enginefacade.dispose_global)


@debtcollector.removals.removed_class("DbTestCase")
class DbTestCase(test_base.BaseTestCase):
"""Base class for testing of DB code.
Expand Down Expand Up @@ -191,6 +194,7 @@ def generate_schema(self, engine):
"implemented within generate_schema().")


@debtcollector.removals.removed_class("OpportunisticTestCase")
class OpportunisticTestCase(DbTestCase):
"""Placeholder for backwards compatibility."""

Expand Down Expand Up @@ -220,18 +224,22 @@ def ins_wrap(self):
return wrap


@debtcollector.removals.removed_class("MySQLOpportunisticFixture")
class MySQLOpportunisticFixture(DbFixture):
DRIVER = 'mysql'


@debtcollector.removals.removed_class("PostgreSQLOpportunisticFixture")
class PostgreSQLOpportunisticFixture(DbFixture):
DRIVER = 'postgresql'


@debtcollector.removals.removed_class("MySQLOpportunisticTestCase")
class MySQLOpportunisticTestCase(OpportunisticTestCase):
FIXTURE = MySQLOpportunisticFixture


@debtcollector.removals.removed_class("PostgreSQLOpportunisticTestCase")
class PostgreSQLOpportunisticTestCase(OpportunisticTestCase):
FIXTURE = PostgreSQLOpportunisticFixture

Expand Down

0 comments on commit 2ad571c

Please sign in to comment.