mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-11-02 03:33:04 +08:00
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.
168 lines
5.2 KiB
Python
168 lines
5.2 KiB
Python
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from galaxy.config import BaseAppConfiguration
|
|
from galaxy.config.schema import AppSchema
|
|
from galaxy.exceptions import ConfigurationError
|
|
|
|
# When a config property 'foo' has an attribute 'path_resolves_to', that attribute is a reference to
|
|
# another property 'bar'. Together, these two properties form a graph where 'foo' and 'bar are
|
|
# vertices and the reference from 'foo' to 'bar' is a directed edge.
|
|
#
|
|
# A schema may have any number of such implicit graphs, each having one or more edges. All together,
|
|
# they should form a DAG (directed acyclic graph).
|
|
#
|
|
# These tests ensure that the graph is loaded correctly for a variety of valid configurations,
|
|
# whereas an invalid configuration raises an error.
|
|
|
|
|
|
def get_schema(app_mapping):
|
|
return {"mapping": {"_": {"mapping": app_mapping}}}
|
|
|
|
|
|
def test_basecase(monkeypatch):
|
|
# Check that a valid graph is loaded correctly (this graph has 2 components)
|
|
mock_schema = {
|
|
"component1_path0": {
|
|
"type": "str",
|
|
"default": "value0",
|
|
},
|
|
"component1_path1": {
|
|
"type": "str",
|
|
"default": "value1",
|
|
"path_resolves_to": "component1_path0",
|
|
},
|
|
"component1_path2": {
|
|
"type": "str",
|
|
"default": "value2",
|
|
"path_resolves_to": "component1_path1",
|
|
},
|
|
"component2_path0": {
|
|
"type": "str",
|
|
"default": "value3",
|
|
},
|
|
"component2_path1": {
|
|
"type": "str",
|
|
"default": "value4",
|
|
"path_resolves_to": "component2_path0",
|
|
},
|
|
}
|
|
monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema))
|
|
monkeypatch.setattr(BaseAppConfiguration, "_load_schema", lambda a: AppSchema(Path("no path"), "_"))
|
|
|
|
config = BaseAppConfiguration()
|
|
assert config.component1_path0 == "value0"
|
|
assert config.component1_path1 == "value0/value1"
|
|
assert config.component1_path2 == "value0/value1/value2"
|
|
assert config.component2_path0 == "value3"
|
|
assert config.component2_path1 == "value3/value4"
|
|
|
|
|
|
def test_resolves_to_invalid_property(monkeypatch):
|
|
# 'path_resolves_to' should point to an existing property in the schema
|
|
mock_schema = {
|
|
"path0": {
|
|
"type": "str",
|
|
"default": "value0",
|
|
},
|
|
"path1": {
|
|
"type": "str",
|
|
"default": "value1",
|
|
"path_resolves_to": "invalid", # invalid
|
|
},
|
|
}
|
|
monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema))
|
|
|
|
with pytest.raises(ConfigurationError):
|
|
AppSchema(Path("no path"), "_").validate_path_resolution_graph()
|
|
|
|
|
|
def test_path_resolution_cycle(monkeypatch):
|
|
# Must be a DAG, but this one has a cycle
|
|
mock_schema = {
|
|
"path0": {
|
|
"type": "str",
|
|
"default": "value0",
|
|
"path_resolves_to": "path2",
|
|
},
|
|
"path1": {
|
|
"type": "str",
|
|
"default": "value1",
|
|
"path_resolves_to": "path0",
|
|
},
|
|
"path2": {
|
|
"type": "str",
|
|
"default": "value2",
|
|
"path_resolves_to": "path1",
|
|
},
|
|
}
|
|
monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema))
|
|
|
|
with pytest.raises(ConfigurationError):
|
|
AppSchema(Path("no path"), "_").validate_path_resolution_graph()
|
|
|
|
|
|
def test_path_invalid_type(monkeypatch):
|
|
# Paths should be strings
|
|
mock_schema = {
|
|
"path0": {
|
|
"type": "str",
|
|
"default": "value0",
|
|
},
|
|
"path1": {
|
|
"type": "float", # invalid
|
|
"default": "value1",
|
|
"path_resolves_to": "path0",
|
|
},
|
|
}
|
|
monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema))
|
|
|
|
with pytest.raises(ConfigurationError):
|
|
AppSchema(Path("no path"), "_").validate_path_resolution_graph()
|
|
|
|
|
|
def test_resolves_to_invalid_type(monkeypatch):
|
|
# Paths should be strings
|
|
mock_schema = {
|
|
"path0": {
|
|
"type": "int", # invalid
|
|
"default": "value0",
|
|
},
|
|
"path1": {
|
|
"type": "str",
|
|
"default": "value1",
|
|
"path_resolves_to": "path0",
|
|
},
|
|
}
|
|
monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema))
|
|
|
|
with pytest.raises(ConfigurationError):
|
|
AppSchema(Path("no path"), "_").validate_path_resolution_graph()
|
|
|
|
|
|
def test_resolves_with_empty_component(monkeypatch):
|
|
# A path can be None (root path is never None; may be assigned elsewhere)
|
|
mock_schema = {
|
|
"path0": {
|
|
"type": "str",
|
|
"default": "value0",
|
|
},
|
|
"path1": {
|
|
"type": "str",
|
|
"path_resolves_to": "path0",
|
|
},
|
|
"path2": {
|
|
"type": "str",
|
|
"default": "value2",
|
|
"path_resolves_to": "path0",
|
|
},
|
|
}
|
|
monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema))
|
|
monkeypatch.setattr(BaseAppConfiguration, "_load_schema", lambda a: AppSchema(Path("no path"), "_"))
|
|
|
|
config = BaseAppConfiguration()
|
|
assert config.path0 == "value0"
|
|
assert config.path1 is None
|
|
assert config.path2 == "value0/value2"
|