From c07cfc66550c252b08415399ce8146e1a20adadc Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Fri, 16 Oct 2015 17:48:51 +0100 Subject: [PATCH] Enable migration autogenerate These changes allow a developer to generate migrations using the autogenerate function without the need to pass in a config file that includes sql connection information. Change-Id: I6b3942f7747e8f73e52925c24340e20daeb78911 --- CONTRIBUTING.rst | 42 ++++++++++++++++++++++++------ ironic_inspector/db.py | 21 +++++++++------ ironic_inspector/dbsync.py | 6 ++--- ironic_inspector/migrations/env.py | 7 +++-- ironic_inspector/test/base.py | 9 ++----- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 165cf82ca..05329496f 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -239,16 +239,15 @@ Writing a Plugin .. _ironic_inspector.plugins.base: https://github.com/openstack/ironic-inspector/blob/master/ironic_inspector/plugins/base.py .. _Introspection Rules: https://github.com/openstack/ironic-inspector#introspection-rules -Adding migrations to the database -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Making changes to the database +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -In order to make any changes to the database, you must add a new migration. -This can be done using alembic:: +In order to make a change to the ironic-inspector database you must update the +database models found in ironic_inspector.db_ and then create a migration to +reflect that change. - alembic --config ironic_inspector/alembic.ini revision -m "A short description" - -This will generate an empty migration file, with the correct revision -information already included. In this file there are two functions: +There are two ways to create a migration which are described below, both of +these generate a new migration file. In this file there are two functions: * upgrade - The upgrade function is run when ``ironic-inspector-dbsync upgrade`` is run, and should be populated with @@ -262,4 +261,31 @@ information already included. In this file there are two functions: For further information on creating a migration, refer to `Create a Migration Script`_ from the alembic documentation. +Autogenerate +------------ + +This is the simplest way to create a migration. Alembic will compare the models +to an up to date database, and then attempt to write a migration based on the +differences. This should generate correct migrations in most cases however +there are some cases when it can not detect some changes and may require +manual modification, see `What does Autogenerate Detect (and what does it not +detect?)`_ from the alembic documentation. + +:: + + ironic-inspector-dbsync upgrade + ironic-inspector-dbsync revision -m "A short description" --autogenerate + +Manual +------ + +This will generate an empty migration file, with the correct revision +information already included. However upgrade and downgrade are left empty and +must be manually populated in order to perform the correct actions on the +database:: + + ironic-inspector-dbsync revision -m "A short description" + .. _Create a Migration Script: https://alembic.readthedocs.org/en/latest/tutorial.html#create-a-migration-script +.. _ironic_inspector.db: https://github.com/openstack/ironic-inspector/blob/master/ironic_inspector/db.py +.. _What does Autogenerate Detect (and what does it not detect?): http://alembic.readthedocs.org/en/latest/autogenerate.html#what-does-autogenerate-detect-and-what-does-it-not-detect diff --git a/ironic_inspector/db.py b/ironic_inspector/db.py index 0af56b01a..3ddc7d130 100644 --- a/ironic_inspector/db.py +++ b/ironic_inspector/db.py @@ -27,6 +27,8 @@ from sqlalchemy import (Boolean, Column, DateTime, Float, ForeignKey, Integer, from sqlalchemy.ext.declarative import declarative_base from sqlalchemy import orm +from ironic_inspector import conf # noqa + class ModelBase(models.ModelBase): __table_args__ = {'mysql_engine': "InnoDB", @@ -35,9 +37,16 @@ class ModelBase(models.ModelBase): Base = declarative_base(cls=ModelBase) CONF = cfg.CONF - +_DEFAULT_SQL_CONNECTION = 'sqlite:///ironic_inspector.sqlite' _FACADE = None +db_opts.set_defaults(cfg.CONF, _DEFAULT_SQL_CONNECTION, + 'ironic_inspector.sqlite') +if CONF.discoverd.database: + db_opts.set_defaults(CONF, + connection='sqlite:///%s' % + str(CONF.discoverd.database).strip()) + class Node(Base): __tablename__ = 'nodes' @@ -110,20 +119,16 @@ class RuleAction(Base): def init(): """Initialize the database.""" - if CONF.discoverd.database: - db_opts.set_defaults(CONF, - connection='sqlite:///%s' % - str(CONF.discoverd.database).strip()) return get_session() def get_session(**kwargs): - facade = _create_facade_lazily() + facade = create_facade_lazily() return facade.get_session(**kwargs) def get_engine(): - facade = _create_facade_lazily() + facade = create_facade_lazily() return facade.get_engine() @@ -138,7 +143,7 @@ def model_query(model, *args, **kwargs): return query -def _create_facade_lazily(): +def create_facade_lazily(): global _FACADE if _FACADE is None: _FACADE = db_session.EngineFacade.from_config(cfg.CONF) diff --git a/ironic_inspector/dbsync.py b/ironic_inspector/dbsync.py index b3d197073..01375f84d 100644 --- a/ironic_inspector/dbsync.py +++ b/ironic_inspector/dbsync.py @@ -22,13 +22,11 @@ from alembic import config as alembic_config from alembic import util as alembic_util from oslo_config import cfg -from oslo_db import options as db_opts from oslo_log import log from ironic_inspector import conf # noqa CONF = cfg.CONF -db_opts.set_defaults(CONF) def add_alembic_command(subparsers, name): @@ -53,6 +51,7 @@ def add_command_parsers(subparsers): parser = add_alembic_command(subparsers, 'revision') parser.set_defaults(func=do_revision) parser.add_argument('-m', '--message') + parser.add_argument('--autogenerate', action='store_true') command_opt = cfg.SubCommandOpt('command', @@ -64,7 +63,8 @@ CONF.register_cli_opt(command_opt) def do_revision(config, cmd, *args, **kwargs): - do_alembic_command(config, cmd, message=CONF.command.message) + do_alembic_command(config, cmd, message=CONF.command.message, + autogenerate=CONF.command.autogenerate) def with_revision(config, cmd, *args, **kwargs): diff --git a/ironic_inspector/migrations/env.py b/ironic_inspector/migrations/env.py index c0796ef5b..3c9c88a1f 100644 --- a/ironic_inspector/migrations/env.py +++ b/ironic_inspector/migrations/env.py @@ -14,7 +14,8 @@ from alembic import context from logging.config import fileConfig -from sqlalchemy import create_engine + +from ironic_inspector import db # this is the Alembic Config object, which provides # access to the values within the .ini file in use. @@ -29,7 +30,6 @@ fileConfig(config.config_file_name) # for 'autogenerate' support # from myapp import mymodel # target_metadata = mymodel.Base.metadata -from ironic_inspector import db target_metadata = db.Base.metadata # other values from the config, defined by the needs of env.py, @@ -65,8 +65,7 @@ def run_migrations_online(): and associate a connection with the context. """ - connectable = create_engine(ironic_inspector_config.database.connection) - + connectable = db.create_facade_lazily().get_engine() with connectable.connect() as connection: context.configure( connection=connection, diff --git a/ironic_inspector/test/base.py b/ironic_inspector/test/base.py index 348086a52..bce3798aa 100644 --- a/ironic_inspector/test/base.py +++ b/ironic_inspector/test/base.py @@ -15,7 +15,6 @@ import unittest import mock from oslo_config import cfg -from oslo_db import options as db_opts from oslo_log import log from ironic_inspector.common import i18n @@ -35,7 +34,7 @@ def init_test_conf(): # Unit tests except Exception: CONF.reset() - for group in ('firewall', 'processing', 'ironic'): + for group in ('firewall', 'processing', 'ironic', 'discoverd'): CONF.register_group(cfg.OptGroup(group)) try: # Functional tests @@ -43,13 +42,9 @@ def init_test_conf(): except Exception: # Unit tests pass - db_opts.set_defaults(CONF) + CONF.set_default('connection', "sqlite:///", group='database') CONF.set_default('slave_connection', False, group='database') CONF.set_default('max_retries', 10, group='database') - if not CONF.database.connection: - # Might be set in functional tests - db_opts.set_defaults(CONF, - connection='sqlite:///') class BaseTest(unittest.TestCase):