Preserve manage_db.sh, but do not hide alembic

- Preserves usage of legacy script while not hiding the Alembic script.
    (see manage_db.sh and run_alembic.sh)
- Adds documentation (see top of script files)
- Includes misc. relevant refactorings
- Includes unit tests for testing legacy script handling

lib/galaxy/model/migrations/__init__.py

@@ -382,7 +382,7 @@ class DatabaseStateVerifier:
         msg = f'Your {model} database has version {db_version}, but this code expects '
         msg += f'version {code_version}. '
         msg += 'This database can be upgraded automatically if database_auto_migrate is set. '
-        msg += 'To upgrade manually, run `migrate_db.sh` (see instructions in that file). '
+        msg += 'To upgrade manually, run `run_alembic.sh` (see instructions in that file). '
         msg += 'Please remember to backup your database before migrating.'
         return msg
 

lib/galaxy/model/migrations/scripts.py

@@ -1,4 +1,5 @@
 import os
+import re
 from typing import (
     List,
     NamedTuple,
@@ -60,3 +61,77 @@ def get_configuration(argv: List[str], cwd: str) -> Tuple[DatabaseConfig, Databa
     tsi_config = DatabaseConfig(url, template, encoding)
 
     return (gxy_config, tsi_config, is_auto_migrate)
+
+
+def add_db_urls_to_command_arguments(argv: List[str], cwd: str) -> None:
+    gxy_config, tsi_config, _ = get_configuration(argv, cwd)
+    _insert_x_argument(argv, 'tsi_url', tsi_config.url)
+    _insert_x_argument(argv, 'gxy_url', gxy_config.url)
+
+
+def _insert_x_argument(argv, key: str, value: str) -> None:
+    # `_insert_x_argument('mykey', 'myval')` transforms `foo -a 1` into `foo -x mykey=myval -a 42`
+    argv.insert(1, f'{key}={value}')
+    argv.insert(1, '-x')
+
+
+class LegacyScripts:
+
+    LEGACY_CONFIG_FILE_ARG_NAMES = ['-c', '--config', '--config-file']
+    ALEMBIC_CONFIG_FILE_ARG = '--alembic-config'  # alembic config file, set in the calling script
+
+    def pop_database_argument(self, argv: List[str]) -> str:
+        """
+        If last argument is a valid database name, pop and return it; otherwise return default.
+        """
+        arg = argv[-1]
+        if arg in ['galaxy', 'install']:
+            return argv.pop()
+        return 'galaxy'
+
+    def rename_config_argument(self, argv: List[str]) -> None:
+        """
+        Rename the optional config argument: we can't use '-c' because that option is used by Alembic.
+        """
+        for arg in self.LEGACY_CONFIG_FILE_ARG_NAMES:
+            if arg in argv:
+                self._rename_arg(argv, arg, CONFIG_FILE_ARG)
+                return
+
+    def rename_alembic_config_argument(self, argv: List[str]) -> None:
+        """
+        Rename argument name: `--alembic-config` to `-c`. There should be no `-c` argument present.
+        """
+        if '-c' in argv:
+            raise Exception('Cannot rename alembic config argument: `-c` argument present.')
+        self._rename_arg(argv, self.ALEMBIC_CONFIG_FILE_ARG, '-c')
+
+    def convert_version_argument(self, argv: List[str], database: str) -> None:
+        """
+        Convert legacy version argument to current spec required by Alembic.
+        """
+        if '--version' in argv:
+            # Just remove it: the following argument should be the version/revision identifier.
+            pos = argv.index('--version')
+            argv.pop(pos)
+        else:
+            # If we find --version=foo, extract foo and replace arg with foo (which is the revision identifier)
+            p = re.compile(r'--version=([0-9A-Fa-f]+)')
+            for i, arg in enumerate(argv):
+                m = p.match(arg)
+                if m:
+                    argv[i] = m.group(1)
+                    return
+            # No version argumen found: construct branch@head argument for an upgrade operation.
+            # Raise exception otherwise.
+            if 'upgrade' not in argv:
+                raise Exception('If no `--version` argument supplied, `upgrade` argument is requried')
+
+            if database == 'galaxy':
+                argv.append('gxy@head')
+            elif database == 'install':
+                argv.append('tsi@head')
+
+    def _rename_arg(self, argv, old_name, new_name) -> None:
+        pos = argv.index(old_name)
+        argv[pos] = new_name

lib/galaxy/model/orm/scripts.py

@@ -153,7 +153,7 @@ def get_config(argv, use_argparse=True, cwd=None):
 
 def manage_db():
     # This is a duplicate implementation of scripts/migrate_db.py.
-    # See migrate_db.sh for usage.
+    # See run_alembic.sh for usage.
     def _insert_x_argument(key, value):
         sys.argv.insert(1, f'{key}={value}')
         sys.argv.insert(1, '-x')

lib/tool_shed/webapp/model/migrate/check.py

@@ -34,7 +34,7 @@ def create_or_verify_database(url, engine_options=None):
     1) Empty database --> initialize with latest version and return
     2) Database older than migration support --> fail and require manual update
     3) Database at state where migrate support introduced --> add version control information but make no changes (might still require manual update)
-    4) Database versioned but out of date --> fail with informative message, user must run "sh migrate_toolshed_db.sh upgrade"
+    4) Database versioned but out of date --> fail with informative message, user must run "sh manage_db.sh upgrade"
 
     """
     engine_options = engine_options or {}
@@ -81,7 +81,7 @@ def create_or_verify_database(url, engine_options=None):
             migrate_repository.versions.latest,
         )
         exception_msg += "Back up your database and then migrate the schema by running the following from your Galaxy installation directory:"
-        exception_msg += "\n\nsh migrate_toolshed_db.sh upgrade tool_shed\n"
+        exception_msg += "\n\nsh manage_db.sh upgrade tool_shed\n"
         raise Exception(exception_msg)
     else:
         log.info("At database version %d" % db_schema.version)

manage_db.sh

@@ -0,0 +1,49 @@
+#!/bin/sh
+
+#######
+# The purpose of this script is to preserve the legacy interface provided by
+# manage_db.sh, which was a thin wrapper around SQLAlchemy Migrate. Unless you
+# need to use the legacy interface, to manage migrations of the "galaxy" and
+# "install" databases, you are encouraged to use run_alembic.sh directly and
+# take advantage of Alembic's command line options.
+#
+# Use this script to upgrade or downgrade your database.
+# Database options: galaxy (default), install, tool_shed
+# To pass a galaxy config file, you may use `-c|--config|--config-file your-config-file`
+
+# To upgrade or downgrade to some version X:
+# sh manage_db.sh [upgrade|downgrade] --version=X [tool_shed|install|galaxy]
+#
+# You may also skip the version argument when upgrading, in which case the database
+# will be upgraded to the latest version.
+#
+# Example 1: upgrade "galaxy" database to version "abc123" using default config:
+# sh manage_db.sh upgrade --version=xyz567
+#
+# Example 2: downgrade "install" database to version "xyz789" passing config file "mygalaxy.yml":
+# sh manage_db.sh downgrade --version=abc123 -c mygalaxy.yml install
+#
+# Example 3: upgrade "galaxy" database to latest version using defualt config:
+# sh manage_db.sh upgrade
+#
+# (Note: Tool Shed migrations use the legacy migrations system, so we check the
+# last argument (the database) to invoke the appropriate script. Therefore, if
+# you don't specify the database (galaxy is used by default) and pass a config
+# file, your config file should not be named `tool_shed`.)
+#######
+
+ALEMBIC_CONFIG='lib/galaxy/model/migrations/alembic.ini'
+
+cd `dirname $0`
+
+. ./scripts/common_startup_functions.sh
+
+setup_python
+
+for i; do :; done
+if [ "$i" == "tool_shed" ]; then
+    python ./scripts/migrate_toolshed_db.py $@ tool_shed
+else
+    find lib/galaxy/model/migrations/alembic -name '*.pyc' -delete
+    python ./scripts/manage_db_adapter.py --alembic-config "$ALEMBIC_CONFIG" $@
+fi

migrate_toolshed_db.sh

@@ -1,18 +0,0 @@
-#!/bin/sh
-
-#######
-# Use this script to manage Tool Shed migrations.
-# (Use migrate_db.sh to manage Galaxy and Tool Shed Install migrations.)
-#
-# To downgrade to a specific version, use something like:
-# sh manage_toolshed_db.sh downgrade --version=3 tool_shed
-#######
-
-cd `dirname $0`
-
-. ./scripts/common_startup_functions.sh
-
-setup_python
-
-find lib/galaxy/model/migrate/versions -name '*.pyc' -delete
-python ./scripts/migrate_toolshed_db.py $@ tool_shed

run_alembic.sh

@@ -2,7 +2,7 @@
 
 #######
 # Use this script to manage Galaxy and Tool Shed Install migrations.
-# (Use migrate_toolshed_db.sh to manage Tool Shed migrations.)
+# (Use the legacy manage_db.sh script to manage Tool Shed migrations.)
 #
 # NOTE: If your database is empty OR is not under Alembic version control,
 # use create_db.sh instead.
@@ -12,31 +12,31 @@
 # Use these identifiers: `gxy` for galaxy, and `tsi` for tool_shed_install.
 #
 # To create a revision for galaxy:
-# ./migrate_db.sh revision --head=gxy@head -m "your description"
+# ./run_alembic.sh revision --head=gxy@head -m "your description"
 #
 # To create a revision for tool_shed_install:
-# ./migrate_db.sh revision --head=tsi@head -m "your description"
+# ./run_alembic.sh revision --head=tsi@head -m "your description"
 #
 # To upgrade:
-# ./migrate_db.sh upgrade gxy@head  # upgrade gxy to head revision
-# ./migrate_db.sh upgrade gxy@+1  # upgrade gxy to 1 revision above current
-# ./migrate_db.sh upgrade [revision identifier]  # upgrade gxy to a specific revision
-# ./migrate_db.sh upgrade [revision identifier]+1  # upgrade gxy to 1 revision above specific revision
-# ./migrate_db.sh upgrade heads  # upgrade gxy and tsi to head revisions
+# ./run_alembic.sh upgrade gxy@head  # upgrade gxy to head revision
+# ./run_alembic.sh upgrade gxy@+1  # upgrade gxy to 1 revision above current
+# ./run_alembic.sh upgrade [revision identifier]  # upgrade gxy to a specific revision
+# ./run_alembic.sh upgrade [revision identifier]+1  # upgrade gxy to 1 revision above specific revision
+# ./run_alembic.sh upgrade heads  # upgrade gxy and tsi to head revisions
 #
 # To downgrade:
-# ./migrate_db.sh downgrade gxy@base  # downgrade gxy to base (empty db with empty alembic table)
-# ./migrate_db.sh downgrade gxy@-1  # downgrade gxy to 1 revision below current
-# ./migrate_db.sh downgrade [revision identifier]  # downgrade gxy to a specific revision
-# ./migrate_db.sh downgrade [revision identifier]-1  # downgrade gxy to 1 revision below specific revision
+# ./run_alembic.sh downgrade gxy@base  # downgrade gxy to base (empty db with empty alembic table)
+# ./run_alembic.sh downgrade gxy@-1  # downgrade gxy to 1 revision below current
+# ./run_alembic.sh downgrade [revision identifier]  # downgrade gxy to a specific revision
+# ./run_alembic.sh downgrade [revision identifier]-1  # downgrade gxy to 1 revision below specific revision
 #
 # 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 ./migrate_db.sh ...
-# GALAXY_INSTALL_CONFIG_OVERRIDE_DATABASE_CONNECTION=my-other-db-url ./migrate_db.sh ...
+# GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION=my-db-url ./run_alembic.sh ...
+# GALAXY_INSTALL_CONFIG_OVERRIDE_DATABASE_CONNECTION=my-other-db-url ./run_alembic.sh ...
 #
 # For more options, see Alembic's documentation at https://alembic.sqlalchemy.org
 #######

scripts/manage_db_adapter.py

@@ -0,0 +1,43 @@
+"""
+This script is intended to be invoked by the legacy manage_db.sh script.
+It translates the arguments supplied to manage_db.sh into the format used
+by migrate_db.py.
+
+INPUT:                        |   OUTPUT:
+----------------------------------------------------------
+upgrade --version=foo         |   upgrade foo
+upgrade --version foo         |   upgrade foo
+upgrade                       |   upgrade gxy@head
+upgrade install               |   upgrade tsi@head
+upgrade --version=bar install |   upgrade bar
+upgrade -c path-to-galaxy.yml |   upgrade --galaxy-config path-to-galaxy.yml gxy@head
+
+The converted sys.argv will include `-c path-to-alembic.ini`.
+The optional `-c` argument name is renamed to `--galaxy-config`.
+"""
+
+import os
+import sys
+
+import alembic.config
+
+sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib')))
+
+from galaxy.model.migrations.scripts import (
+    add_db_urls_to_command_arguments,
+    LegacyScripts,
+)
+
+
+def run():
+    ls = LegacyScripts()
+    target_database = ls.pop_database_argument(sys.argv)
+    ls.rename_config_argument(sys.argv)
+    ls.rename_alembic_config_argument(sys.argv)
+    ls.convert_version_argument(sys.argv, target_database)
+    add_db_urls_to_command_arguments(sys.argv, os.getcwd())
+    alembic.config.main()
+
+
+if __name__ == '__main__':
+    run()

scripts/migrate_db.py

@@ -1,7 +1,8 @@
 """
 This script retrieves relevant configuration values and invokes
 the Alembic console runner.
-It is wrapped by migrate_db.sh (see that file for usage).
+It is wrapped by run_alembic.sh (see that file for usage) and by
+manage_db.sh (for legacy usage).
 """
 import logging
 import os.path
@@ -12,14 +13,14 @@ import alembic.config
 sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib')))
 
 from galaxy.model.migrations import GXY, TSI
-from galaxy.model.migrations.scripts import get_configuration
+from galaxy.model.migrations.scripts import add_db_urls_to_command_arguments
 
 logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger(__name__)
 
 
 def invoke_alembic():
-    _add_db_urls_to_command_arguments()
+    add_db_urls_to_command_arguments(sys.argv, os.getcwd())
 
     # Accept 'heads' as the target revision argument to enable upgrading both gxy and tsi in one command.
     # This is consistent with Alembic's CLI, which allows `upgrade heads`. However, this would not work for
@@ -35,17 +36,5 @@ def invoke_alembic():
         alembic.config.main()
 
 
-def _add_db_urls_to_command_arguments():
-    gxy_config, tsi_config, _ = get_configuration(sys.argv, os.getcwd())
-    _insert_x_argument('tsi_url', tsi_config.url)
-    _insert_x_argument('gxy_url', gxy_config.url)
-
-
-def _insert_x_argument(key, value):
-    # `_insert_x_argument('mykey', 'myval')` transforms `foo -a 1` into `foo -x mykey=myval -a 42`
-    sys.argv.insert(1, f'{key}={value}')
-    sys.argv.insert(1, '-x')
-
-
 if __name__ == '__main__':
     invoke_alembic()

scripts/migrate_toolshed_db.py

@@ -2,7 +2,7 @@
 This script parses the Tool Shed config file for database connection
 and then delegates to sqlalchemy_migrate shell main function in
 migrate.versioning.shell.
-It is wrapped by migrate_toolshed_db.sh (see that file for usage).
+It is wrapped by manage_db.sh (see that file for usage).
 """
 import logging
 import os.path

test/unit/data/model/migrations/test_scripts.py

@@ -0,0 +1,81 @@
+import pytest
+
+from galaxy.model.migrations.scripts import LegacyScripts
+
+
+class TestLegacyScripts():
+
+    def test_pop_database_name__pop_and_return(self):
+        arg_value = 'install'
+        argv = ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc', arg_value]
+        database = LegacyScripts().pop_database_argument(argv)
+        assert database == arg_value
+        assert argv == ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc']
+
+    def test_pop_database_name__pop_and_return_default(self):
+        arg_value = 'galaxy'
+        argv = ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc', arg_value]
+        database = LegacyScripts().pop_database_argument(argv)
+        assert database == arg_value
+        assert argv == ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc']
+
+    def test_pop_database_name__return_default(self):
+        argv = ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc']
+        database = LegacyScripts().pop_database_argument(argv)
+        assert database == 'galaxy'
+        assert argv == ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc']
+
+    @pytest.mark.parametrize('arg_name', LegacyScripts.LEGACY_CONFIG_FILE_ARG_NAMES)
+    def test_rename_config_arg(self, arg_name):
+        # `-c|--config|__config-file` should be renamed to `--galaxy-config`
+        argv = ['caller', '--alembic-config', 'path-to-alembic', arg_name, 'path-to-galaxy', 'upgrade', '--version=abc']
+        LegacyScripts().rename_config_argument(argv)
+        assert argv == ['caller', '--alembic-config', 'path-to-alembic', '--galaxy-config', 'path-to-galaxy', 'upgrade', '--version=abc']
+
+    def test_rename_config_arg_reordered_args(self):
+        # `-c|--config|__config-file` should be renamed to `--galaxy-config`
+        argv = ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc', '-c', 'path-to-galaxy']
+        LegacyScripts().rename_config_argument(argv)
+        assert argv == ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc', '--galaxy-config', 'path-to-galaxy']
+
+    def test_rename_alembic_config_arg(self):
+        # `--alembic-config` should be renamed to `-c`
+        argv = ['caller', '--alembic-config', 'path-to-alembic', 'upgrade', '--version=abc']
+        LegacyScripts().rename_alembic_config_argument(argv)
+        assert argv == ['caller', '-c', 'path-to-alembic', 'upgrade', '--version=abc']
+
+    def test_rename_alembic_config_arg_raises_error_if_c_arg_present(self):
+        # Ensure alembic config arg is renamed AFTER renaming the galaxy config arg. Raise error otherwise.
+        argv = ['caller', '--alembic-config', 'path-to-alembic', '-c', 'path-to-galaxy', 'upgrade', '--version=abc']
+        with pytest.raises(Exception):
+            LegacyScripts().rename_alembic_config_argument(argv)
+
+    def test_convert_version_argument_1(self):
+        # `--version foo` should be converted to `foo`
+        argv = ['caller', '-c', 'path-to-alembic', 'upgrade', '--version', 'abc']
+        LegacyScripts().convert_version_argument(argv, 'galaxy')
+        assert argv == ['caller', '-c', 'path-to-alembic', 'upgrade', 'abc']
+
+    def test_convert_version_argument_2(self):
+        # `--version=foo` should be converted to `foo`
+        argv = ['caller', '-c', 'path-to-alembic', 'upgrade', '--version=abc']
+        LegacyScripts().convert_version_argument(argv, 'galaxy')
+        assert argv == ['caller', '-c', 'path-to-alembic', 'upgrade', 'abc']
+
+    def test_no_version_argument(self):
+        # No version should be converted to `X@head` where `X` is either gxy or tsi, depending on the target database.
+        database = 'galaxy'
+        argv = ['caller', '-c', 'path-to-alembic', 'upgrade']
+        LegacyScripts().convert_version_argument(argv, database)
+        assert argv == ['caller', '-c', 'path-to-alembic', 'upgrade', 'gxy@head']
+
+        database = 'install'
+        argv = ['caller', '-c', 'path-to-alembic', 'upgrade']
+        LegacyScripts().convert_version_argument(argv, database)
+        assert argv == ['caller', '-c', 'path-to-alembic', 'upgrade', 'tsi@head']
+
+    def test_downgrade_with_no_version_argument_raises_error(self):
+        database = 'galaxy'
+        argv = ['caller', '-c', 'path-to-alembic', 'downgrade']
+        with pytest.raises(Exception):
+            LegacyScripts().convert_version_argument(argv, database)