Only run sqlalchemy-migrate migrations if we have to; separate foundations
authorChristopher Allan Webber <cwebber@dustycloud.org>
Sat, 12 Mar 2016 17:45:28 +0000 (09:45 -0800)
committerChristopher Allan Webber <cwebber@dustycloud.org>
Sat, 26 Mar 2016 18:39:07 +0000 (11:39 -0700)
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
mediagoblin/gmg_commands/dbupdate.py

index 2706c9b73bf360f4f78a9da5dce036e31118ac7b..f4273fa0e37ce30293288e1fd244a2380441fa48 100644 (file)
@@ -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
index e9f05e01e657aedfeb435caa977c506dd8aca613..8ec78c7331ee88b964baa96d86fa94fd68325cc2 100644 (file)
@@ -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)