Integrate migrations design into config and model code

lib/galaxy/config/__init__.py

@@ -42,8 +42,8 @@
 from galaxy.exceptions import ConfigurationError
 from galaxy.model import mapping
 from galaxy.model.database_utils import database_exists
-from galaxy.model.tool_shed_install.migrate.check import create_or_verify_database as tsi_create_or_verify_database
 from galaxy.schema.fields import BaseDatabaseIdField
+from galaxy.model.orm.engine_factory import build_engine
 from galaxy.structured_app import BasicSharedApp
 from galaxy.util import (
     ExecutionTimer,
@@ -1476,58 +1476,66 @@ def _configure_tool_shed_registry(self):
         else:
             self.tool_shed_registry = galaxy.tool_shed.tool_shed_registry.Registry()
 
+    def _is_one_database(self, db_url, install_db_url):
+        # TODO: Consider more aggressive check here that this is not the same
+        # database file under the hood.
+        return not(db_url and install_db_url and install_db_url != db_url)
+
+    def _configure_engines(self, db_url, install_db_url, combined_install_database):
+        trace_logger = getattr(self, "trace_logger", None)
+        engine = build_engine(
+            db_url,
+            self.config.database_engine_options,
+            self.config.database_query_profiling_proxy,
+            trace_logger,
+            self.config.slow_query_log_threshold,
+            self.config.thread_local_log,
+            self.config.database_log_query_counts
+        )
+        install_engine = None
+        if not combined_install_database:
+            install_engine = build_engine(install_db_url, self.config.install_database_engine_options)
+        return engine, install_engine
+
     def _configure_models(self, check_migrate_databases=False, config_file=None):
         """Preconditions: object_store must be set on self."""
+        # TODO this block doesn't seem to belong in this method
+        if getattr(self.config, "max_metadata_value_size", None):
+            from galaxy.model import custom_types
+            custom_types.MAX_METADATA_VALUE_SIZE = self.config.max_metadata_value_size
+
         db_url = get_database_url(self.config)
         install_db_url = self.config.install_database_connection
-        # TODO: Consider more aggressive check here that this is not the same
-        # database file under the hood.
-        combined_install_database = not (install_db_url and install_db_url != db_url)
-        install_db_url = install_db_url or db_url
-        install_database_options = (
-            self.config.database_engine_options
-            if combined_install_database
-            else self.config.install_database_engine_options
-        )
+        combined_install_database = self._is_one_database(db_url, install_db_url)
+        engine, install_engine = self._configure_engines(db_url, install_db_url, combined_install_database)
 
         if self.config.database_wait:
             self._wait_for_database(db_url)
 
-        if getattr(self.config, "max_metadata_value_size", None):
-            from galaxy.model import custom_types
+        if check_migrate_databases:
+            from galaxy.model.migrations import verify_databases
+            verify_databases(engine, install_engine, self.config)
 
-            custom_types.MAX_METADATA_VALUE_SIZE = self.config.max_metadata_value_size
+        self.model = self._this_should_be_inlined(engine, combined_install_database)
 
-        if check_migrate_databases:
-            # Initialize database / check for appropriate schema version.  # If this
-            # is a new installation, we'll restrict the tool migration messaging.
-            from galaxy.model.migrate.check import create_or_verify_database
-
-            create_or_verify_database(
-                db_url,
-                config_file,
-                self.config.database_engine_options,
-                app=self,
-                map_install_models=combined_install_database,
-            )
-            if not combined_install_database:
-                tsi_create_or_verify_database(install_db_url, install_database_options, app=self)
-
-        self.model = init_models_from_config(
-            self.config,
-            map_install_models=combined_install_database,
-            object_store=self.object_store,
-            trace_logger=getattr(self, "trace_logger", None),
-        )
         if combined_install_database:
-            log.info("Install database targetting Galaxy's database configuration.")
+            log.info("Install database targeting Galaxy's database configuration.")  # TODO this message is ambiguous
             self.install_model = self.model
         else:
             from galaxy.model.tool_shed_install import mapping as install_mapping
-
-            install_db_url = self.config.install_database_connection
+            self.install_model = install_mapping.configure_model_mapping(install_engine)
             log.info(f"Install database using its own connection {install_db_url}")
-            self.install_model = install_mapping.init(install_db_url, install_database_options)
+
+    def _this_should_be_inlined(self, engine, combined_install_database):
+        # TODO this is WIP: see comments in method above.
+        return mapping.configure_model_mapping(
+            self.config.file_path,
+            self.object_store,
+            self.config.use_pbkdf2,
+            engine,
+            combined_install_database,
+            self.config.thread_local_log
+        )
 
     def _configure_signal_handlers(self, handlers):
         for sig, handler in handlers.items():

lib/galaxy/model/mapping.py

@@ -1,9 +1,3 @@
-"""
-This module no longer contains the mapping of data model classes to the
-relational database.
-The module will be revised during migration from SQLAlchemy Migrate to Alembic.
-"""
-
 import logging
 from threading import local
 from typing import (
@@ -14,9 +8,9 @@
 from galaxy import model
 from galaxy.model import mapper_registry
 from galaxy.model.base import SharedModelMapping
-from galaxy.model.migrate.triggers.update_audit_table import install as install_timestamp_triggers
 from galaxy.model.orm.engine_factory import build_engine
 from galaxy.model.security import GalaxyRBACAgent
+from galaxy.model.triggers.update_audit_table import install as install_timestamp_triggers
 from galaxy.model.view.utils import install_views
 
 log = logging.getLogger(__name__)
@@ -27,63 +21,61 @@
 class GalaxyModelMapping(SharedModelMapping):
     security_agent: GalaxyRBACAgent
     thread_local_log: Optional[local]
-    create_tables: bool
     User: Type
     GalaxySession: Type
 
 
-def init(
+def init(file_path, url, engine_options=None, create_tables=False, map_install_models=False,
+        database_query_profiling_proxy=False, object_store=None, trace_logger=None, use_pbkdf2=True,
+        slow_query_log_threshold=0, thread_local_log: Optional[local] = None, log_query_counts=False) -> GalaxyModelMapping:
+    # Build engine
+    engine = build_engine(
+        url, engine_options, database_query_profiling_proxy, trace_logger, slow_query_log_threshold,
+        thread_local_log=thread_local_log, log_query_counts=log_query_counts)
+
+    # Create tables if needed
+    if create_tables:
+        mapper_registry.metadata.create_all(bind=engine)
+        create_additional_database_objects(engine)
+        if map_install_models:
+            from galaxy.model.tool_shed_install import mapping as install_mapping  # noqa: F401
+            install_mapping.create_database_objects(engine)
+
+    # Configure model, build ModelMapping
+    return configure_model_mapping(
+        file_path, object_store, use_pbkdf2, engine, map_install_models, thread_local_log)
+
+
+def create_additional_database_objects(engine):
+    install_timestamp_triggers(engine)
+    install_views(engine)
+
+
+def configure_model_mapping(
     file_path,
-    url,
-    engine_options=None,
-    create_tables=False,
-    map_install_models=False,
-    database_query_profiling_proxy=False,
-    object_store=None,
-    trace_logger=None,
-    use_pbkdf2=True,
-    slow_query_log_threshold=0,
-    thread_local_log: Optional[local] = None,
-    log_query_counts=False,
+    object_store,
+    use_pbkdf2,
+    engine,
+    map_install_models,
+    thread_local_log,
 ) -> GalaxyModelMapping:
-    """Connect mappings to the database"""
-    if engine_options is None:
-        engine_options = {}
-    # Connect dataset to the file path
+    _configure_model(file_path, object_store, use_pbkdf2)
+    return _build_model_mapping(engine, map_install_models, thread_local_log)
+
+
+def _configure_model(file_path, object_store, use_pbkdf2):
     model.Dataset.file_path = file_path
-    # Connect dataset to object store
     model.Dataset.object_store = object_store
-    # Use PBKDF2 password hashing?
     model.User.use_pbkdf2 = use_pbkdf2
-    # Load the appropriate db module
-    engine = build_engine(
-        url,
-        engine_options,
-        database_query_profiling_proxy,
-        trace_logger,
-        slow_query_log_threshold,
-        thread_local_log=thread_local_log,
-        log_query_counts=log_query_counts,
-    )
 
+
+def _build_model_mapping(engine, map_install_models, thread_local_log):
     model_modules = [model]
     if map_install_models:
-        import galaxy.model.tool_shed_install.mapping  # noqa: F401
         from galaxy.model import tool_shed_install
-
-        galaxy.model.tool_shed_install.mapping.init(url=url, engine_options=engine_options, create_tables=create_tables)
         model_modules.append(tool_shed_install)
 
-    result = GalaxyModelMapping(model_modules, engine=engine)
-
-    # Create tables if needed
-    if create_tables:
-        metadata.create_all(bind=engine)
-        install_timestamp_triggers(engine)
-        install_views(engine)
-
-    result.create_tables = create_tables
-    # load local galaxy security policy
-    result.security_agent = GalaxyRBACAgent(result)
-    result.thread_local_log = thread_local_log
-    return result
+    model_mapping = GalaxyModelMapping(model_modules, engine=engine)
+    model_mapping.security_agent = GalaxyRBACAgent(model_mapping)
+    model_mapping.thread_local_log = thread_local_log
+    return model_mapping

lib/galaxy/model/tool_shed_install/mapping.py

@@ -7,15 +7,16 @@
 
 
 def init(url, engine_options=None, create_tables=False):
-    """Connect mappings to the database"""
-    # Load the appropriate db module
-    engine_options = engine_options or {}
     engine = build_engine(url, engine_options)
-    result = ModelMapping([install_model], engine=engine)
-    # Create tables if needed
     if create_tables:
-        metadata.create_all(bind=engine)
-        # metadata.engine.commit()
-    result.create_tables = create_tables
-    # load local galaxy security policy
-    return result
+        create_database_objects(engine)
+    return configure_model_mapping(engine)
+
+
+def create_database_objects(engine):
+    mapper_registry.metadata.create_all(bind=engine)
+
+
+def configure_model_mapping(engine):
+    # TODO: do we need to load local galaxy security policy?
+    return ModelMapping([install_model], engine=engine)