From 300dd45a55d39964e2db1169f8c716a869e06325 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 13 Dec 2024 17:02:37 +0000 Subject: [PATCH] Fix startup when ``data_manager_config_file`` config option is set to an empty value by not resolving it to the `path_resolves_to` directory in such a case. planemo (which we use in the converters tests) set this option to an empty value when not testing data managers: https://github.com/galaxyproject/planemo/blob/master/planemo/galaxy/config.py#L452 Resulting in this traceback: ``` Traceback (most recent call last): File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/webapps/galaxy/buildapp.py", line 68, in app_pair app = galaxy.app.UniverseApplication(global_conf=global_conf, is_webapp=True, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/app.py", line 777, in __init__ self.data_managers = self._register_singleton(DataManagers) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/di.py", line 27, in _register_singleton instance = self[dep_type] ~~~~^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 381, in __getitem__ return self.resolve(dep) ^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 265, in resolve return self._reflection_build_with_err_handling(dep_type, suppress_error) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 392, in _reflection_build_with_err_handling return self._reflection_build(dep_type) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 408, in _reflection_build return dep_type(**sub_deps) # type: ignore ^^^^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/tools/data_manager/manager.py", line 44, in __init__ self.load_from_xml(filename) File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/tools/data_manager/manager.py", line 58, in load_from_xml tree = util.parse_xml(xml_filename) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/util/__init__.py", line 360, in parse_xml with source.open("rb") as f: ^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.12/pathlib.py", line 1013, in open return io.open(self, mode, buffering, encoding, errors, newline) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ IsADirectoryError: [Errno 21] Is a directory: '/tmp/tmps2qq5htq' ``` Why this has become a problem ----------------------------- Before commit 3282aa3a56e9dafd279ee2fd48aead06725bfb2c `util.parse_xml()` would raise a `XMLSyntaxError` exception if passed a directory, but now it raises `IsADirectoryError`, which is a subclass of `OSError` with errno 21. As a consequence when `load_from_xml()` in lib/galaxy/tools/data_manager/manager.py calls `util.parse_xml()` passing a directory, the resulting exception is now re-raised in the `except OSError as e:` branch instead of just being logged, which is good since this is a misconfiguration. N.B.: This will also need to be fixed in Planemo by not setting the `data_manager_config_file` option when not needed. --- lib/galaxy/config/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index 5cae18bf10bd..8e4ce9ca23ff 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -301,7 +301,7 @@ def is_set(self, key): # To check only schema options, change the line below to `if property not in self._raw_config:` if key not in self._raw_config: log.warning(f"Configuration option does not exist: '{key}'") - return key in self._kwargs + return key in self._kwargs and self._kwargs[key] def resolve_path(self, path): """Resolve a path relative to Galaxy's root.""" @@ -511,10 +511,10 @@ def resolve(key): if not parent: # base case: nothing else needs resolving return path parent_path = resolve(parent) # recursively resolve parent path - if path is not None: + if path: path = os.path.join(parent_path, path) # resolve path else: - path = parent_path # or use parent path + log.warning("Trying to resolve path for the '%s' option but it's empty/None", key) setattr(self, key, path) # update property _cache[key] = path # cache it!