Skip to content

Commit

Permalink
Improve error message when SA Migrate version is incorrect
Browse files Browse the repository at this point in the history
See inline comment ("version too old" is misleading if the incorrect
version is more recent than what the system expects).
  • Loading branch information
jdavcs committed Mar 11, 2022
1 parent 84d5787 commit c8ee55e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
22 changes: 15 additions & 7 deletions lib/galaxy/model/migrations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,20 @@ def __init__(self, model: str) -> None:
super().__init__(f"Your {model} database has no version table; manual update is required")


class VersionTooOldError(Exception):
# The database has a SQLAlchemy Migrate version table, but its version is older than
# {SQLALCHEMYMIGRATE_LAST_VERSION_GXY/TSI}, so it cannot be upgraded with Alembic.
class IncorrectVersionError(Exception):
# The database has a SQLAlchemy Migrate version table, but its version is either older or more recent
# than {SQLALCHEMYMIGRATE_LAST_VERSION_GXY/TSI}, so it cannot be upgraded with Alembic.
# (A more recent version may indicate that something has changed in the database past the point
# where we can automatically migrate from SQLAlchemy Migrate to Alembic.)
# Manual update required.
def __init__(self, model: str) -> None:
super().__init__(f"Your {model} database version is too old; manual update is required")
if model == GXY:
expected_version = SQLALCHEMYMIGRATE_LAST_VERSION_GXY
else:
expected_version = SQLALCHEMYMIGRATE_LAST_VERSION_TSI
msg = f"Your {model} database version is incorrect; version {expected_version} is expected."
msg += "Manual update is required"
super().__init__(msg)


class OutdatedDatabaseError(Exception):
Expand Down Expand Up @@ -358,7 +366,7 @@ def _handle_nonempty_database(self) -> None:
if self._is_last_sqlalchemymigrate_version():
self._try_to_upgrade()
else:
self._handle_version_too_old()
self._handle_wrong_sqlalchemymigrate_version()
else:
self._handle_no_version_table()

Expand Down Expand Up @@ -458,9 +466,9 @@ def _handle_no_version_table(self) -> NoReturn:
model = self._get_model_name()
raise NoVersionTableError(model)

def _handle_version_too_old(self) -> NoReturn:
def _handle_wrong_sqlalchemymigrate_version(self) -> NoReturn:
model = self._get_model_name()
raise VersionTooOldError(model)
raise IncorrectVersionError(model)


def get_last_sqlalchemymigrate_version(model: ModelId) -> int:
Expand Down
4 changes: 2 additions & 2 deletions lib/galaxy/model/migrations/scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
DatabaseConfig,
DatabaseStateCache,
GXY,
IncorrectVersionError,
SQLALCHEMYMIGRATE_LAST_VERSION_GXY,
TSI,
VersionTooOldError,
)
from galaxy.util.properties import (
find_config_file,
Expand Down Expand Up @@ -239,7 +239,7 @@ def get_gxy_db_version(self, gxy_db_url=None):
if not version:
version = self._get_gxy_sam_db_version(engine)
if version != SQLALCHEMYMIGRATE_LAST_VERSION_GXY:
raise VersionTooOldError(GXY)
raise IncorrectVersionError(GXY)
return version
finally:
engine.dispose()
Expand Down
12 changes: 6 additions & 6 deletions test/unit/data/model/migrations/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
DatabaseStateVerifier,
get_last_sqlalchemymigrate_version,
GXY,
IncorrectVersionError,
listify,
load_metadata,
NoVersionTableError,
Expand All @@ -24,7 +25,6 @@
SQLALCHEMYMIGRATE_TABLE,
TSI,
verify_databases,
VersionTooOldError,
)
from galaxy.model.migrations.scripts import LegacyManageDb
from .common import ( # noqa: F401 (url_factory is a fixture we have to import explicitly)
Expand Down Expand Up @@ -423,17 +423,17 @@ class TestState2:
# the stored version is not the latest after which we could transition to Alembic.
# Expect: fail with appropriate message.
def test_combined_database(self, db_state2_combined):
with pytest.raises(VersionTooOldError):
with pytest.raises(IncorrectVersionError):
with disposing_engine(db_state2_combined) as engine:
_verify_databases(engine)

def test_separate_databases_gxy_raises_error(self, db_state2_gxy, db_state6_tsi):
with pytest.raises(VersionTooOldError):
with pytest.raises(IncorrectVersionError):
with disposing_engine(db_state2_gxy) as engine1, disposing_engine(db_state6_tsi) as engine2:
_verify_databases(engine1, engine2)

def test_separate_databases_tsi_raises_error(self, db_state6_gxy, db_state2_tsi):
with pytest.raises(VersionTooOldError):
with pytest.raises(IncorrectVersionError):
with disposing_engine(db_state6_gxy) as engine1, disposing_engine(db_state2_tsi) as engine2:
_verify_databases(engine1, engine2)

Expand Down Expand Up @@ -1108,14 +1108,14 @@ class TestState2:
def test_get_gxy_db_version__state2__gxy_database(self, db_state2_gxy):
# Expect: fail
db_url = db_state2_gxy
with pytest.raises(VersionTooOldError):
with pytest.raises(IncorrectVersionError):
mdb = LegacyManageDb()
mdb.get_gxy_db_version(db_url)

def test_get_gxy_db_version__state2__combined_database(self, db_state2_combined):
# Expect: fail
db_url = db_state2_combined
with pytest.raises(VersionTooOldError):
with pytest.raises(IncorrectVersionError):
mdb = LegacyManageDb()
mdb.get_gxy_db_version(db_url)

Expand Down

0 comments on commit c8ee55e

Please sign in to comment.