Merge pull request #13513 from ic4f/dev_alembic20_rebase2_1

Reorganized alembic branch

.github/workflows/first_startup.yaml

@@ -57,9 +57,6 @@ jobs:
         yarn-lock-file: 'galaxy root/client/yarn.lock'
     - name: Install tox
       run: pip install tox
-      # Use this job to test the latest migrations
-    - run: wget -q https://github.com/jmchilton/galaxy-downloads/raw/master/db_gx_rev_0141.sqlite
-    - run: mv db_gx_rev_0141.sqlite 'galaxy root'/database/universe.sqlite
     - name: run tests
       run: tox -e first_startup
       working-directory: 'galaxy root'

.github/workflows/unit-postgres.yaml

@@ -0,0 +1,56 @@
+name: Unit w/postgres tests
+on:
+  push:
+    paths-ignore:
+      - 'client/**'
+      - 'doc/**'
+  pull_request:
+    paths-ignore:
+      - 'client/**'
+      - 'doc/**'
+env:
+  GALAXY_TEST_DBURI: 'postgresql://postgres:postgres@localhost:5432/postgres?client_encoding=utf8'  # using postgres as the db
+concurrency:
+  group: py-unit-${{ github.ref }}
+  cancel-in-progress: true
+jobs:
+  test:
+    name: Test
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: ['3.7']
+    services:
+      postgres:
+        image: postgres:13
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: postgres
+        ports:
+          - 5432:5432
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          path: 'galaxy root'
+      - uses: actions/setup-python@v2
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Get full Python version
+        id: full-python-version
+        shell: bash
+        run: echo ::set-output name=version::$(python -c "import sys; print('-'.join(str(v) for v in sys.version_info))")
+      - name: Cache pip dir
+        uses: actions/cache@v2
+        with:
+          path: ~/.cache/pip
+          key: pip-cache-${{ matrix.python-version }}-${{ hashFiles('galaxy root/requirements.txt') }}
+      - name: Cache galaxy venv
+        uses: actions/cache@v2
+        with:
+          path: .venv
+          key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-unitdb
+      - name: Run tests
+        run: ./run_tests.sh -unit test/unit/data/model/migrations/test_migrations.py
+        working-directory: 'galaxy root'

create_db.sh

@@ -1,5 +1,21 @@
 #!/bin/sh
 
+#######
+# Use this script to verify the state of the Galaxy and Tool Shed Install
+# database(s). If the database does not exist or is empty, it will be created
+# and initialized.
+# (Use create_toolshed_db.sh to create and initialize a new
+# Tool Shed database.)
+#
+# To pass a galaxy config file, use `--galaxy-config`
+#
+# You may also override the galaxy database url and/or the
+# tool shed install database url, as well as the database_template
+# and database_encoding configuration options with env vars:
+# GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION=my-db-url ./create_db.sh
+# GALAXY_INSTALL_CONFIG_OVERRIDE_DATABASE_CONNECTION=my-other-db-url ./create_db.sh
+#######
+
 cd "$(dirname "$0")"
 
 . ./scripts/common_startup_functions.sh

create_toolshed_db.sh

@@ -0,0 +1,16 @@
+#!/bin/sh
+
+#######
+# Use this script to verify the state of the Tool Shed database.
+# If the database does not exist or is empty, it will be created
+# and initialized.
+# (For Galaxy and Tool Shed Install databases, use create_db.sh).
+#######
+
+cd "$(dirname "$0")"
+
+. ./scripts/common_startup_functions.sh
+
+setup_python
+
+python ./scripts/create_toolshed_db.py "$@" tool_shed

lib/galaxy/app.py

@@ -50,22 +50,25 @@
     WorkflowContentsManager,
     WorkflowsManager,
 )
-from galaxy.model import custom_types
+from galaxy.model import (
+    custom_types,
+    mapping,
+)
 from galaxy.model.base import SharedModelMapping
 from galaxy.model.database_heartbeat import DatabaseHeartbeat
-from galaxy.model.database_utils import database_exists
-from galaxy.model.mapping import (
-    GalaxyModelMapping,
-    init_models_from_config,
+from galaxy.model.database_utils import (
+    database_exists,
+    is_one_database,
 )
-from galaxy.model.migrate.check import create_or_verify_database
+from galaxy.model.mapping import GalaxyModelMapping
+from galaxy.model.migrations import verify_databases
+from galaxy.model.orm.engine_factory import build_engine
 from galaxy.model.scoped_session import (
     galaxy_scoped_session,
     install_model_scoped_session,
 )
 from galaxy.model.tags import GalaxyTagHandler
 from galaxy.model.tool_shed_install import mapping as install_mapping
-from galaxy.model.tool_shed_install.migrate.check import create_or_verify_database as tsi_create_or_verify_database
 from galaxy.objectstore import build_object_store_from_config
 from galaxy.queue_worker import (
     GalaxyQueueWorker,
@@ -334,52 +337,70 @@ def _configure_tool_shed_registry(self):
         else:
             self.tool_shed_registry = tool_shed_registry.Registry()
 
+    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):
+            custom_types.MAX_METADATA_VALUE_SIZE = self.config.max_metadata_value_size
+
         db_url = self.config.database_connection
         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 = 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):
-            custom_types.MAX_METADATA_VALUE_SIZE = self.config.max_metadata_value_size
-
         if check_migrate_databases:
-            # Initialize database / check for appropriate schema version.  # If this
-            # is a new installation, we'll restrict the tool migration messaging.
-            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),
+            self._verify_databases(engine, install_engine, combined_install_database)
+
+        self.model = mapping.configure_model_mapping(
+            self.config.file_path,
+            self.object_store,
+            self.config.use_pbkdf2,
+            engine,
+            combined_install_database,
+            self.config.thread_local_log,
         )
+
         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:
-            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 _verify_databases(self, engine, install_engine, combined_install_database):
+        install_template, install_encoding = None, None
+        if not combined_install_database:  # Otherwise these options are not used.
+            install_template = getattr(self.config, "install_database_template", None)
+            install_encoding = getattr(self.config, "install_database_encoding", None)
+
+        verify_databases(
+            engine,
+            self.config.database_template,
+            self.config.database_encoding,
+            install_engine,
+            install_template,
+            install_encoding,
+            self.config.database_auto_migrate,
+        )
 
     def _configure_signal_handlers(self, handlers):
         for sig, handler in handlers.items():
@@ -419,7 +440,6 @@ def __init__(self, fsmon=False, **kwargs) -> None:
         log.debug("python path is: %s", ", ".join(sys.path))
         self.name = "galaxy"
         self.is_webapp = False
-        self.new_installation = False
         # Read config file and check for errors
         self.config: Any = self._register_singleton(config.Configuration, config.Configuration(**kwargs))
         self.config.check()

lib/galaxy/config/__init__.py

@@ -774,7 +774,6 @@ def _process_config(self, kwargs):
             db_path = self._in_data_dir("universe.sqlite")
             self.database_connection = f"sqlite:///{db_path}?isolation_level=IMMEDIATE"
         self.database_engine_options = get_database_engine_options(kwargs)
-        self.database_create_tables = string_as_bool(kwargs.get("database_create_tables", "True"))
         self.database_encoding = kwargs.get("database_encoding")  # Create new databases with this encoding
         self.thread_local_log = None
         if self.enable_per_request_sql_debugging:

lib/galaxy/dependencies/dev-requirements.txt

@@ -4,6 +4,7 @@ a2wsgi==1.4.0; python_version >= "3.6" and python_version < "4.0"
 adal==1.2.7
 aiofiles==0.8.0; python_version >= "3.6" and python_version < "4.0"
 alabaster==0.7.12; python_version >= "3.6" and python_full_version < "3.0.0" or python_full_version >= "3.4.0" and python_version >= "3.6"
+alembic==1.7.4; python_version >= "3.6"
 amqp==5.0.9; python_version >= "3.7"
 anyio==3.5.0; python_version >= "3.7" and python_full_version >= "3.6.2"
 appdirs==1.4.4

lib/galaxy/dependencies/pinned-requirements.txt

@@ -3,6 +3,7 @@
 a2wsgi==1.4.0; python_version >= "3.6" and python_version < "4.0"
 adal==1.2.7
 aiofiles==0.8.0; python_version >= "3.6" and python_version < "4.0"
+alembic==1.7.4; python_version >= "3.6"
 amqp==5.0.9; python_version >= "3.7"
 anyio==3.5.0; python_version >= "3.7" and python_full_version >= "3.6.2"
 appdirs==1.4.4

lib/galaxy/model/database_utils.py

@@ -1,5 +1,6 @@
 import sqlite3
 from contextlib import contextmanager
+from typing import Optional
 
 from sqlalchemy import create_engine
 from sqlalchemy.engine.url import make_url
@@ -118,3 +119,14 @@ def create(self, encoding, *arg):
             stmt = f"CREATE DATABASE {database} CHARACTER SET = '{encoding}'"
             with engine.connect().execution_options(isolation_level="AUTOCOMMIT") as conn:
                 conn.execute(stmt)
+
+
+def is_one_database(db1_url: str, db2_url: Optional[str]):
+    """
+    Check if the arguments refer to one database. This will be true
+    if only one argument is passed, or if the urls are the same.
+    URLs are strings, so sameness is determined via string comparison.
+    """
+    # TODO: Consider more aggressive check here that this is not the same
+    # database file under the hood.
+    return not (db1_url and db2_url and db1_url != db2_url)