Skip to content

Commit

Permalink
Remove circular dependency between migrations and config
Browse files Browse the repository at this point in the history
  • Loading branch information
jdavcs committed Mar 9, 2022
1 parent c842973 commit a54be22
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 67 deletions.
16 changes: 14 additions & 2 deletions lib/galaxy/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1510,8 +1510,7 @@ def _configure_models(self, check_migrate_databases=False, config_file=None):
self._wait_for_database(db_url)

if check_migrate_databases:
from galaxy.model.migrations import verify_databases
verify_databases(engine, install_engine, self.config)
self._verify_databases(engine, install_engine, combined_install_database)

self.model = mapping.configure_model_mapping(
self.config.file_path,
Expand All @@ -1530,6 +1529,19 @@ def _configure_models(self, check_migrate_databases=False, config_file=None):
self.install_model = install_mapping.configure_model_mapping(install_engine)
log.info(f"Install database using its own connection {install_db_url}")

def _verify_databases(self, engine, install_engine, combined_install_database):
from galaxy.model.migrations import verify_databases
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():
signal.signal(sig, handler)
Expand Down
36 changes: 2 additions & 34 deletions lib/galaxy/model/migrations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,8 @@
Engine,
)

from galaxy.config import GalaxyAppConfiguration
from galaxy.model import Base as gxy_base
from galaxy.model.database_utils import (
create_database,
database_exists,
is_one_database,
)
from galaxy.model.database_utils import create_database, database_exists
from galaxy.model.mapping import create_additional_database_objects
from galaxy.model.migrations.scripts import DatabaseConfig
from galaxy.model.tool_shed_install import Base as tsi_base
Expand Down Expand Up @@ -244,41 +239,14 @@ def verify_databases_via_script(
if tsi_config.url and tsi_config.url != gxy_config.url:
tsi_engine = create_engine(tsi_config.url)

_verify(
verify_databases(
gxy_engine, gxy_config.template, gxy_config.encoding,
tsi_engine, tsi_config.template, tsi_config.encoding,
is_auto_migrate
)


def verify_databases(
gxy_engine: Engine,
tsi_engine: Optional[Engine] = None,
config: Optional[GalaxyAppConfiguration] = None
) -> None:
gxy_template, gxy_encoding = None, None
tsi_template, tsi_encoding = None, None
is_auto_migrate = False

if config:
is_auto_migrate = config.database_auto_migrate
gxy_template = config.database_template
gxy_encoding = config.database_encoding

is_combined = gxy_engine and tsi_engine and \
is_one_database(str(gxy_engine.url), str(tsi_engine.url))
if not is_combined: # Otherwise not used.
tsi_template = getattr(config, 'install_database_template', None)
tsi_encoding = getattr(config, 'install_database_encoding', None)

_verify(
gxy_engine, gxy_template, gxy_encoding,
tsi_engine, tsi_template, tsi_encoding,
is_auto_migrate
)


def _verify(
gxy_engine: Engine,
gxy_template: Optional[str],
gxy_encoding: Optional[str],
Expand Down
66 changes: 35 additions & 31 deletions test/unit/data/model/migrations/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ def test_database_combined(self, db_state6_gxy_state3_tsi_no_sam, metadata_state
assert AlembicManagerForTests.is_at_revision(engine, GXY_REVISION_2)


def _verify_databases(gxy_engine, tsi_engine=None):
verify_databases(gxy_engine, None, None, tsi_engine, None, None, False)


class TestDatabaseStates:
# Tests of primary function under different scenarios and database state.

Expand All @@ -360,7 +364,7 @@ def test_combined_database(self, url_factory, metadata_state6_combined): # noqa
with drop_database(db_url):
assert not database_exists(db_url)
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -370,7 +374,7 @@ def test_separate_databases(self, url_factory, metadata_state6_gxy, metadata_sta
assert not database_exists(db2_url)
with drop_database(db1_url), drop_database(db2_url):
with disposing_engine(db1_url) as engine1, disposing_engine(db2_url) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)
assert database_is_up_to_date(db1_url, metadata_state6_gxy, GXY)
assert database_is_up_to_date(db2_url, metadata_state6_tsi, TSI)

Expand All @@ -383,7 +387,7 @@ def test_combined_database(self, url_factory, metadata_state6_combined): # noqa
with disposing_engine(db_url) as engine:
assert database_exists(db_url)
assert database_is_empty(db_url)
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -395,7 +399,7 @@ def test_separate_databases(self, url_factory, metadata_state6_gxy, metadata_sta
assert database_exists(db2_url)
assert database_is_empty(db1_url)
assert database_is_empty(db2_url)
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)
assert database_is_up_to_date(db1_url, metadata_state6_gxy, GXY)
assert database_is_up_to_date(db2_url, metadata_state6_tsi, TSI)

Expand All @@ -405,17 +409,17 @@ class TestState1:
def test_combined_database(self, db_state1_combined):
with pytest.raises(NoVersionTableError):
with disposing_engine(db_state1_combined) as engine:
verify_databases(engine)
_verify_databases(engine)

def test_separate_databases_gxy_raises_error(self, db_state1_gxy, db_state6_tsi):
with pytest.raises(NoVersionTableError):
with disposing_engine(db_state1_gxy) as engine1, disposing_engine(db_state6_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

def test_separate_databases_tsi_raises_error(self, db_state6_gxy, db_state1_tsi):
with pytest.raises(NoVersionTableError):
with disposing_engine(db_state6_gxy) as engine1, disposing_engine(db_state1_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

class TestState2:
# Initial state: non-empty database, SQLAlchemy Migrate version table present; however,
Expand All @@ -424,17 +428,17 @@ class TestState2:
def test_combined_database(self, db_state2_combined):
with pytest.raises(VersionTooOldError):
with disposing_engine(db_state2_combined) as engine:
verify_databases(engine)
_verify_databases(engine)

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

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

class TestState3:
# Initial state: non-empty database, SQLAlchemy Migrate version table contains latest version
Expand All @@ -450,7 +454,7 @@ def test_combined_database_automigrate(
):
db_url = db_state3_combined
with disposing_engine(db_state3_combined) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -464,24 +468,24 @@ def test_separate_databases_automigrate(
):
db1_url, db2_url = db_state3_gxy, db_state3_tsi
with disposing_engine(db1_url) as engine1, disposing_engine(db2_url) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)
assert database_is_up_to_date(db1_url, metadata_state6_gxy, GXY)
assert database_is_up_to_date(db2_url, metadata_state6_tsi, TSI)

def test_combined_database_no_automigrate(self, db_state3_combined):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state3_combined) as engine:
verify_databases(engine)
_verify_databases(engine)

def test_separate_databases_no_automigrate_gxy_raises_error(self, db_state3_gxy, db_state6_tsi):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state3_gxy) as engine1, disposing_engine(db_state6_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

def test_separate_databases_no_automigrate_tsi_raises_error(self, db_state6_gxy, db_state3_tsi):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state6_gxy) as engine1, disposing_engine(db_state3_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

class TestState4:
# Initial state: non-empty database, SQLAlchemy Migrate version table present, Alembic version table present.
Expand All @@ -497,7 +501,7 @@ def test_combined_database_automigrate(
):
db_url = db_state4_combined
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -511,24 +515,24 @@ def test_separate_databases_automigrate(
):
db1_url, db2_url = db_state4_gxy, db_state4_tsi
with disposing_engine(db1_url) as engine1, disposing_engine(db2_url) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)
assert database_is_up_to_date(db1_url, metadata_state6_gxy, GXY)
assert database_is_up_to_date(db2_url, metadata_state6_tsi, TSI)

def test_combined_database_no_automigrate(self, db_state4_combined):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state4_combined) as engine:
verify_databases(engine)
_verify_databases(engine)

def test_separate_databases_no_automigrate_gxy_raises_error(self, db_state4_gxy, db_state6_tsi):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state4_gxy) as engine1, disposing_engine(db_state6_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

def test_separate_databases_no_automigrate_tsi_raises_error(self, db_state6_gxy, db_state4_tsi):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state6_gxy) as engine1, disposing_engine(db_state4_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

class TestState5:
# Initial state: non-empty database, Alembic version table present.
Expand All @@ -544,7 +548,7 @@ def test_combined_database_automigrate(
):
db_url = db_state5_combined
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -558,32 +562,32 @@ def test_separate_databases_automigrate(
):
db1_url, db2_url = db_state5_gxy, db_state5_tsi
with disposing_engine(db1_url) as engine1, disposing_engine(db2_url) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)
assert database_is_up_to_date(db1_url, metadata_state6_gxy, GXY)
assert database_is_up_to_date(db2_url, metadata_state6_tsi, TSI)

def test_combined_database_no_automigrate(self, db_state5_combined):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state5_combined) as engine:
verify_databases(engine)
_verify_databases(engine)

def test_separate_databases_no_automigrate_gxy_raises_error(self, db_state5_gxy, db_state6_tsi):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state5_gxy) as engine1, disposing_engine(db_state6_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

def test_separate_databases_no_automigrate_tsi_raises_error(self, db_state6_gxy, db_state5_tsi):
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_state6_gxy) as engine1, disposing_engine(db_state5_tsi) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)

class TestState6:
# Initial state: non-empty database, Alembic version table present, database up-to-date.
# Expect: do nothing.
def test_combined_database(self, db_state6_combined, metadata_state6_combined):
db_url = db_state6_combined
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -596,7 +600,7 @@ def test_separate_databases(
):
db1_url, db2_url = db_state6_gxy, db_state6_tsi
with disposing_engine(db1_url) as engine1, disposing_engine(db2_url) as engine2:
verify_databases(engine1, engine2)
_verify_databases(engine1, engine2)
assert database_is_up_to_date(db1_url, metadata_state6_gxy, GXY)
assert database_is_up_to_date(db2_url, metadata_state6_tsi, TSI)

Expand Down Expand Up @@ -627,7 +631,7 @@ def test_case1_automigrate(self, db_state6_gxy, metadata_state6_combined, set_au
# Expect: database is up-to-date
db_url = db_state6_gxy
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -638,7 +642,7 @@ def test_case1_no_automigrate(self, db_state6_gxy, metadata_state6_combined):
# Expect: database is up-to-date (same as auto-migrate enabled)
db_url = db_state6_gxy
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -649,7 +653,7 @@ def test_case2_automigrate(self, db_state6_gxy_state3_tsi_no_sam, metadata_state
# Expect: database is up-to-date
db_url = db_state6_gxy_state3_tsi_no_sam
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)
assert database_is_up_to_date(db_url, metadata_state6_combined, GXY)
assert database_is_up_to_date(db_url, metadata_state6_combined, TSI)

Expand All @@ -661,7 +665,7 @@ def test_case2_no_automigrate(self, db_state6_gxy_state3_tsi_no_sam):
db_url = db_state6_gxy_state3_tsi_no_sam
with pytest.raises(OutdatedDatabaseError):
with disposing_engine(db_url) as engine:
verify_databases(engine)
_verify_databases(engine)


# Test helpers + their tests, misc. fixtures
Expand Down

0 comments on commit a54be22

Please sign in to comment.