From f25b476202b5ebe706a99fa586041c23b85acaa5 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 12 Mar 2016 09:45:28 -0800 Subject: [PATCH] Only run sqlalchemy-migrate migrations if we have to; separate foundations The goal is to get things to the point where Alembic can run on its own for new databases and initialize the whole database on its own. There are risks to not doing so, see #5413 for details. There's a lot more here that could removed or cleaned up once sqlalchemy-migrate is *completely* removed in the future. * mediagoblin/db/migration_tools.py (MigrationManager.foundations): Removed attribute. (MigrationManager.populate_table_foundations): Removed method. (MigrationManager.init_or_migrate): Removed call to deprecated method. (populate_table_foundations): New function, refactored from former MigrationManager method of same name. * mediagoblin/gmg_commands/dbupdate.py: Import populate_table_foundations. (DatabaseData.foundations): Remove attribute. (DatabaseData.make_migration_manager): Adjust instantiation of MigrationManager. (gather_database_data): Move out the work of building up foundations data. (run_foundations): New method, incorporating logic for gathering and running foundations which was previously spread across other functions and methods. (run_alembic_migrations): Remove deprecated comment. (run_dbupdate): Only run sqlalchemy migrations if we have to. Also run run_foundations if we are setting up this database for the first time. (sqam_migrations_to_run): New method. --- mediagoblin/db/migration_tools.py | 33 +++++---- mediagoblin/gmg_commands/dbupdate.py | 107 +++++++++++++++++++++------ 2 files changed, 103 insertions(+), 37 deletions(-) diff --git a/mediagoblin/db/migration_tools.py b/mediagoblin/db/migration_tools.py index 2706c9b7..f4273fa0 100644 --- a/mediagoblin/db/migration_tools.py +++ b/mediagoblin/db/migration_tools.py @@ -44,7 +44,7 @@ class MigrationManager(object): to the latest migrations, etc. """ - def __init__(self, name, models, foundations, migration_registry, session, + def __init__(self, name, models, migration_registry, session, printer=simple_printer): """ Args: @@ -55,7 +55,6 @@ class MigrationManager(object): """ self.name = name self.models = models - self.foundations = foundations self.session = session self.migration_registry = migration_registry self._sorted_migrations = None @@ -156,18 +155,6 @@ class MigrationManager(object): self.session.bind, tables=[model.__table__ for model in self.models]) - def populate_table_foundations(self): - """ - Create the table foundations (default rows) as layed out in FOUNDATIONS - in mediagoblin.db.models - """ - for Model, rows in self.foundations.items(): - self.printer(u' + Laying foundations for %s table\n' % - (Model.__name__)) - for parameters in rows: - new_row = Model(**parameters) - self.session.add(new_row) - def create_new_migration_record(self): """ Create a new migration record for this migration set @@ -232,7 +219,6 @@ class MigrationManager(object): # auto-set at latest migration number self.create_new_migration_record() self.printer(u"done.\n") - self.populate_table_foundations() self.set_current_migration() return u'inited' @@ -356,6 +342,23 @@ def model_iteration_hack(db, query): return db.execute(query) +def populate_table_foundations(session, foundations, name, + printer=simple_printer): + """ + Create the table foundations (default rows) as layed out in FOUNDATIONS + in mediagoblin.db.models + """ + printer(u'Laying foundations for %s:\n' % name) + for Model, rows in foundations.items(): + printer(u' + Laying foundations for %s table\n' % + (Model.__name__)) + for parameters in rows: + new_row = Model(**parameters) + session.add(new_row) + + session.commit() + + def build_alembic_config(global_config, cmd_options, session): """ Build up a config that the alembic tooling can use based on our diff --git a/mediagoblin/gmg_commands/dbupdate.py b/mediagoblin/gmg_commands/dbupdate.py index e9f05e01..8ec78c73 100644 --- a/mediagoblin/gmg_commands/dbupdate.py +++ b/mediagoblin/gmg_commands/dbupdate.py @@ -22,7 +22,7 @@ from sqlalchemy.orm import sessionmaker from mediagoblin.db.open import setup_connection_and_db_from_config from mediagoblin.db.migration_tools import ( - MigrationManager, build_alembic_config) + MigrationManager, build_alembic_config, populate_table_foundations) from mediagoblin.init import setup_global_and_app_config from mediagoblin.tools.common import import_component @@ -37,15 +37,14 @@ def dbupdate_parse_setup(subparser): class DatabaseData(object): - def __init__(self, name, models, foundations, migrations): + def __init__(self, name, models, migrations): self.name = name self.models = models - self.foundations = foundations self.migrations = migrations def make_migration_manager(self, session): return MigrationManager( - self.name, self.models, self.foundations, self.migrations, session) + self.name, self.models, self.migrations, session) def gather_database_data(plugins): @@ -62,11 +61,10 @@ def gather_database_data(plugins): # Add main first from mediagoblin.db.models import MODELS as MAIN_MODELS from mediagoblin.db.migrations import MIGRATIONS as MAIN_MIGRATIONS - from mediagoblin.db.models import FOUNDATIONS as MAIN_FOUNDATIONS managed_dbdata.append( DatabaseData( - u'__main__', MAIN_MODELS, MAIN_FOUNDATIONS, MAIN_MIGRATIONS)) + u'__main__', MAIN_MODELS, MAIN_MIGRATIONS)) for plugin in plugins: try: @@ -96,19 +94,37 @@ def gather_database_data(plugins): 'forgotten to add it? ({1})'.format(plugin, exc)) migrations = {} + if models: + managed_dbdata.append( + DatabaseData(plugin, models, migrations)) + + return managed_dbdata + + +def run_foundations(db, global_config): + """ + Gather foundations data and run it. + """ + from mediagoblin.db.models import FOUNDATIONS as MAIN_FOUNDATIONS + all_foundations = [(u"__main__", MAIN_FOUNDATIONS)] + + Session = sessionmaker(bind=db.engine) + session = Session() + + plugins = global_config.get('plugins', {}) + + for plugin in plugins: try: foundations = import_component( '{0}.models:FOUNDATIONS'.format(plugin)) + all_foundations.append(foundations) except ImportError as exc: - foundations = {} + continue except AttributeError as exc: - foundations = {} + continue - if models: - managed_dbdata.append( - DatabaseData(plugin, models, foundations, migrations)) - - return managed_dbdata + for name, foundations in all_foundations: + populate_table_foundations(session, foundations, name) def run_alembic_migrations(db, app_config, global_config): @@ -117,14 +133,9 @@ def run_alembic_migrations(db, app_config, global_config): session = Session() cfg = build_alembic_config(global_config, None, session) - # XXX: we need to call this method when we ditch - # sqlalchemy-migrate entirely - # if self.get_current_revision() is None: - # self.init_tables() return command.upgrade(cfg, 'heads') - def run_dbupdate(app_config, global_config): """ Initialize or migrate the database as specified by the config file. @@ -134,14 +145,30 @@ def run_dbupdate(app_config, global_config): """ # Set up the database db = setup_connection_and_db_from_config(app_config, migrations=True) + + # Do we have migrations + should_run_sqam_migrations = db.engine.has_table("core__migrations") and \ + sqam_migrations_to_run(db, app_config, + global_config) + + # Looks like a fresh database! + # (We set up this variable here because doing "run_all_migrations" below + # will change things.) + fresh_database = ( + not db.engine.has_table("core__migrations") and + not db.engine.has_table("alembic_version")) + # Run the migrations - # TODO: rename to run_legacy_migrations - run_all_migrations(db, app_config, global_config) + if should_run_sqam_migrations: + run_all_migrations(db, app_config, global_config) - # TODO: Make this happen regardless of python 2 or 3 once ensured - # to be "safe"! run_alembic_migrations(db, app_config, global_config) + # If this was our first time initializing the database, + # we must lay down the foundations + if fresh_database: + run_foundations(db, global_config) + def run_all_migrations(db, app_config, global_config): """Initialize or migrates a database. @@ -166,6 +193,42 @@ def run_all_migrations(db, app_config, global_config): migration_manager.init_or_migrate() +def sqam_migrations_to_run(db, app_config, global_config): + """ + Check whether any plugins have sqlalchemy-migrate migrations left to run. + + This is a kludge so we can transition away from sqlalchemy-migrate + except where necessary. + """ + # @@: This shares a lot of code with run_all_migrations, but both + # are legacy and will be removed at some point. + + # Gather information from all media managers / projects + dbdatas = gather_database_data( + list(global_config.get('plugins', {}).keys())) + + Session = sessionmaker(bind=db.engine) + + # We can bail out early if it turns out that sqlalchemy-migrate + # was never installed with any migrations + from mediagoblin.db.models import MigrationData + if Session().query(MigrationData).filter_by( + name=u"__main__").first() is None: + return False + + # Setup media managers for all dbdata, run init/migrate and print info + # For each component, create/migrate tables + for dbdata in dbdatas: + migration_manager = dbdata.make_migration_manager(Session()) + if migration_manager.migrations_to_run(): + # If *any* migration managers have migrations to run, + # we'll have to run them. + return True + + # Otherwise, scot free! + return False + + def dbupdate(args): global_config, app_config = setup_global_and_app_config(args.conf_file) run_dbupdate(app_config, global_config) -- 2.25.1