Skip to content

Commit

Permalink
Address existing mypy error (see note)
Browse files Browse the repository at this point in the history
This is a (possible) bug that has existed on dev. To expose it, simply
change this call to init_models_from_config
(https://github.com/galaxyproject/galaxy/blob/release_21.09/lib/galaxy/config/__init__.py#L1302)
to a call directly to mapping.init(). The result will be 6 mypy errors.

The reason is that the mapping.init() method
(https://github.com/galaxyproject/galaxy/blob/release_21.09/lib/galaxy/model/mapping.py#L79)
specifies its return type as GalaxyModelMapping - which results in
several errors of this type:

lib/galaxy/app.py:178: error: Definition of "model" in base class "MinimalApp" is incompatible with definition in base class "ConfiguresGalaxyMixin"

When we call an intermediary function (that does not have a return type
specified), mypy is happy.

This should be addressed in a separate issue/PR. Removing the return
type is the simplest (temporary) solution.
  • Loading branch information
jdavcs committed Jan 6, 2022
1 parent 8589de9 commit 17622a1
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 13 deletions.
20 changes: 8 additions & 12 deletions lib/galaxy/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,14 @@ def _configure_models(self, check_migrate_databases=False, config_file=None):
from galaxy.model.migrations import verify_databases
verify_databases(engine, install_engine, self.config)

self.model = self._this_should_be_inlined(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 targeting Galaxy's database configuration.") # TODO this message is ambiguous
Expand All @@ -1405,17 +1412,6 @@ 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 _this_should_be_inlined(self, engine, combined_install_database):
# TODO this is WIP: see comments in method above.
return mapping.configure_model_mapping(
self.config.file_path,
self.object_store,
self.config.use_pbkdf2,
engine,
combined_install_database,
self.config.thread_local_log
)

def _configure_signal_handlers(self, handlers):
for sig, handler in handlers.items():
signal.signal(sig, handler)
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/model/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def configure_model_mapping(
engine,
map_install_models,
thread_local_log,
) -> GalaxyModelMapping:
):
_configure_model(file_path, object_store, use_pbkdf2)
return _build_model_mapping(engine, map_install_models, thread_local_log)

Expand Down

0 comments on commit 17622a1

Please sign in to comment.