Merge pull request #14329 from jdavcs/release_22.05_dbinit_error

Improve handling of non-existent or uninitialized database when running upgrade script

lib/galaxy/model/migrations/scripts.py

@@ -14,7 +14,10 @@ from alembic.script import ScriptDirectory
 from sqlalchemy import create_engine
 from sqlalchemy.engine import Engine
 
-from galaxy.model.database_utils import is_one_database
+from galaxy.model.database_utils import (
+    database_exists,
+    is_one_database,
+)
 from galaxy.model.migrations import (
     AlembicManager,
     DatabaseConfig,
@@ -38,6 +41,51 @@ GXY_CONFIG_PREFIX = "GALAXY_CONFIG_"
 TSI_CONFIG_PREFIX = "GALAXY_INSTALL_CONFIG_"
 
 
+class DatabaseDoesNotExistError(Exception):
+    def __init__(self, db_url: str) -> None:
+        super().__init__(
+            f"""The database at {db_url} does not exist. You must
+            create and initialize the database before running this script. You
+            can do so by (a) running `create_db.sh`; or by (b) starting Galaxy,
+            in which case Galaxy will create and initialize the database
+            automatically."""
+        )
+
+
+class DatabaseNotInitializedError(Exception):
+    def __init__(self, db_url: str) -> None:
+        super().__init__(
+            f"""The database at {db_url} is empty. You must
+            initialize the database before running this script. You can do so by
+            (a) running `create_db.sh`; or by (b) starting Galaxy, in which case
+            Galaxy will initialize the database automatically."""
+        )
+
+
+def verify_database_is_initialized(db_url: str) -> None:
+    """
+    Intended for use by scripts that run database migrations (manage_db.sh,
+    run_alembic.sh). Those scripts are meant to run on a database that has been
+    initialized with the appropriate metadata (e.g. galaxy or install model).
+
+    This function will raise an error if the database does not exist or has not
+    been initialized*.
+
+    *NOTE: this function cannot determine whether a database has been properly
+    initialized; it can only tell when a database has *not* been initialized.
+    """
+    if not database_exists(db_url):
+        raise DatabaseDoesNotExistError(db_url)
+
+    engine = create_engine(db_url)
+    try:
+        db_state = DatabaseStateCache(engine=engine)
+        if db_state.is_database_empty() or db_state.contains_only_kombu_tables():
+            raise DatabaseNotInitializedError(db_url)
+    finally:
+        engine.dispose()
+
+
 def get_configuration(argv: List[str], cwd: str) -> Tuple[DatabaseConfig, DatabaseConfig, bool]:
     """
     Return a 3-item-tuple with configuration values used for managing databases.
@@ -120,7 +168,15 @@ class LegacyScripts:
     def __init__(self, argv: List[str], cwd: Optional[str] = None) -> None:
         self.argv = argv
         self.cwd = cwd or os.getcwd()
-        self.database = self.DEFAULT_DB_ARG
+        self._database: Optional[str] = None  # Do not assign default value: `None` means we don't know yet.
+
+    @property
+    def database(self):
+        if self._database is None:
+            raise LegacyScriptsException(
+                "Attempt to access identifier of database before processing the script arguments"
+            )
+        return self._database
 
     def run(self) -> None:
         """
@@ -147,8 +203,9 @@ class LegacyScripts:
         If last argument is a valid database name, pop and assign it; otherwise assign default.
         """
         arg = self.argv[-1]
+        self._database = self.DEFAULT_DB_ARG
         if arg in ["galaxy", "install"]:
-            self.database = self.argv.pop()
+            self._database = self.argv.pop()
 
     def rename_config_argument(self) -> None:
         """
@@ -196,6 +253,12 @@ class LegacyScripts:
                 elif self.database == "install":
                     self.argv.append("tsi@head")
 
+    def get_db_url(self):
+        if self.database in ["galaxy", self.DEFAULT_DB_ARG]:
+            return self.gxy_url
+        elif self.database == "install":
+            return self.tsi_url
+
     def _rename_arg(self, old_name, new_name) -> None:
         pos = self.argv.index(old_name)
         self.argv[pos] = new_name

scripts/manage_db_adapter.py

@@ -25,12 +25,15 @@ sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pa
 from galaxy.model.migrations.scripts import (
     invoke_alembic,
     LegacyScripts,
+    verify_database_is_initialized,
 )
 
 
 def run():
     ls = LegacyScripts(sys.argv, os.getcwd())
     ls.run()
+    db_url = ls.get_db_url()
+    verify_database_is_initialized(db_url)
     invoke_alembic()
 
 

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

@@ -1,8 +1,13 @@
+import random
+
 import pytest
 
 from galaxy.model.migrations.scripts import (
+    DatabaseDoesNotExistError,
+    DatabaseNotInitializedError,
     LegacyScripts,
     LegacyScriptsException,
+    verify_database_is_initialized,
 )
 
 
@@ -145,3 +150,25 @@ class TestLegacyScripts:
         argv = ["caller", "--alembic-config", "path-to-alembic", "downgrade"]
         with pytest.raises(LegacyScriptsException):
             LegacyScripts(argv).convert_args()
+
+    def test_access_database_id(self):
+        db = "galaxy"
+        argv = ["caller", "--alembic-config", "path-to-alembic", "upgrade", db]
+        ls = LegacyScripts(argv)
+        ls.run()
+        assert ls.database == db
+
+    def test_access_database_id_before_processing_script_args_raises_error(self):
+        argv = ["caller", "--alembic-config", "path-to-alembic", "upgrade"]
+        with pytest.raises(LegacyScriptsException):
+            LegacyScripts(argv).database
+
+    def test_verify_database_is_init_raises_error_if_no_database(self):
+        nonexistant_path = str(random.random())[2:]
+        db_url = f"sqlite:////{nonexistant_path}"
+        with pytest.raises(DatabaseDoesNotExistError):
+            verify_database_is_initialized(db_url)
+
+    def test_verify_database_is_init_raises_error_if_database_not_initialized(self, sqlite_memory_url):
+        with pytest.raises(DatabaseNotInitializedError):
+            verify_database_is_initialized(sqlite_memory_url)