mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-10-31 18:42:33 +08:00
Nicola Soranzo
18dca18981
This required other changes included here as well: 1) Extend `parse_xml()` to accept a `Traversable` (i.e. the output of `resource_path()`. Open the file before passing it to `etree.parse()`, since lxml's implementation probably doesn't support a `Traversable`. This also simplifies the exception handling, since the workaround for the missing `errno` in the `OSError` exception is not needed any more. ------ 2) Fix Galaxy startup when the ``data_manager_config_file`` config option is set to an empty value When does this happen? ---------------------- 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 This empty config value was resolved to the `path_resolves_to` directory for this option. With the change in 1) above, this results in the following traceback at Galaxy startup: ``` 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 is this a problem now? -------------------------- Before this commit, `util.parse_xml()` was raising a `XMLSyntaxError` exception if passed a directory, but now it raises an `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 actually good since this is a misconfiguration. Solutions -------- - Do not resolve an empty/None path to the `path_resolves_to` directory. - Do not check empty/None paths against root - Adjst unit tests accordingly - Raise an OSError if the `job_config_file` config option is set to null (previously this would be resolved to a directory, also raising an OSError when trying to open that as a file). N.B.: This will also need to be fixed in Planemo by not setting the `data_manager_config_file` option when not needed.