diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index 1dc273bc7240..ef284bae1158 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -512,10 +512,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! @@ -539,7 +539,7 @@ def resolve(key): if self.is_set(key) and self.paths_to_check_against_root and key in self.paths_to_check_against_root: self._check_against_root(key) - def _check_against_root(self, key): + def _check_against_root(self, key: str): def get_path(current_path, initial_path): # if path does not exist and was set as relative: if not self._path_exists(current_path) and not os.path.isabs(initial_path): @@ -558,6 +558,8 @@ def get_path(current_path, initial_path): return current_path current_value = getattr(self, key) # resolved path or list of resolved paths + if not current_value: + return if isinstance(current_value, list): initial_paths = listify(self._raw_config[key], do_strip=True) # initial unresolved paths updated_paths = [] diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index c26aa11dbc0a..ddfb5f01102f 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -387,6 +387,8 @@ def __init__(self, app: MinimalManagerApp): f" release of Galaxy. Please convert to YAML at {self.app.config.job_config_file} or" f" explicitly set `job_config_file` to {job_config_file} to remove this message" ) + if not job_config_file: + raise OSError() if ".xml" in job_config_file: tree = load(job_config_file) job_config_dict = self.__parse_job_conf_xml(tree) diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py index 534868ed01ec..f05ae5c7ab23 100644 --- a/lib/galaxy/tool_util/linters/datatypes.py +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -2,6 +2,7 @@ from typing import ( Set, TYPE_CHECKING, + Union, ) # from galaxy import config @@ -10,15 +11,17 @@ listify, parse_xml, ) +from galaxy.util.resources import resource_path if TYPE_CHECKING: from galaxy.tool_util.lint import LintContext from galaxy.tool_util.parser import ToolSource + from galaxy.util.resources import Traversable -DATATYPES_CONF = os.path.join(os.path.dirname(__file__), "datatypes_conf.xml.sample") +DATATYPES_CONF = resource_path(__package__, "datatypes_conf.xml.sample") -def _parse_datatypes(datatype_conf_path: str) -> Set[str]: +def _parse_datatypes(datatype_conf_path: Union[str, "Traversable"]) -> Set[str]: datatypes = set() tree = parse_xml(datatype_conf_path) root = tree.getroot() diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 7445a06bb3aa..7a0690412c71 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -45,6 +45,7 @@ Optional, overload, Tuple, + TYPE_CHECKING, TypeVar, Union, ) @@ -75,6 +76,7 @@ LXML_AVAILABLE = True try: from lxml import etree + from lxml.etree import DocumentInvalid # lxml.etree.Element is a function that returns a new instance of the # lxml.etree._Element class. This class doesn't have a proper __init__() @@ -123,6 +125,10 @@ def XML(text: Union[str, bytes]) -> Element: XML, ) + class DocumentInvalid(Exception): # type: ignore[no-redef] + pass + + from . import requests from .custom_logging import get_logger from .inflection import Inflector @@ -142,6 +148,9 @@ def shlex_join(split_command): return " ".join(map(shlex.quote, split_command)) +if TYPE_CHECKING: + from galaxy.util.resources import Traversable + inflector = Inflector() log = get_logger(__name__) @@ -333,7 +342,10 @@ def unique_id(KEY_SIZE=128): def parse_xml( - fname: StrPath, strip_whitespace=True, remove_comments=True, schemafname: Union[StrPath, None] = None + fname: Union[StrPath, "Traversable"], + strip_whitespace: bool = True, + remove_comments: bool = True, + schemafname: Union[StrPath, None] = None, ) -> ElementTree: """Returns a parsed xml tree""" parser = None @@ -348,8 +360,10 @@ def parse_xml( schema_root = etree.XML(schema_file.read()) schema = etree.XMLSchema(schema_root) + source = Path(fname) if isinstance(fname, (str, os.PathLike)) else fname try: - tree = cast(ElementTree, etree.parse(str(fname), parser=parser)) + with source.open("rb") as f: + tree = cast(ElementTree, etree.parse(f, parser=parser)) root = tree.getroot() if strip_whitespace: for elem in root.iter("*"): @@ -359,15 +373,10 @@ def parse_xml( elem.tail = elem.tail.strip() if schema: schema.assertValid(tree) - except OSError as e: - if e.errno is None and not os.path.exists(fname): # type: ignore[unreachable] - # lxml doesn't set errno - e.errno = errno.ENOENT # type: ignore[unreachable] - raise except etree.ParseError: log.exception("Error parsing file %s", fname) raise - except etree.DocumentInvalid: + except DocumentInvalid: log.exception("Validation of file %s failed", fname) raise return tree diff --git a/test/unit/config/test_path_graph.py b/test/unit/config/test_path_graph.py index 02f33787081f..77afe7333c55 100644 --- a/test/unit/config/test_path_graph.py +++ b/test/unit/config/test_path_graph.py @@ -142,7 +142,7 @@ def test_resolves_to_invalid_type(monkeypatch): def test_resolves_with_empty_component(monkeypatch): - # A path can be None (root path is never None; may be asigned elsewhere) + # A path can be None (root path is never None; may be assigned elsewhere) mock_schema = { "path0": { "type": "str", @@ -155,7 +155,7 @@ def test_resolves_with_empty_component(monkeypatch): "path2": { "type": "str", "default": "value2", - "path_resolves_to": "path1", + "path_resolves_to": "path0", }, } monkeypatch.setattr(AppSchema, "_read_schema", lambda a, b: get_schema(mock_schema)) @@ -163,5 +163,5 @@ def test_resolves_with_empty_component(monkeypatch): config = BaseAppConfiguration() assert config.path0 == "value0" - assert config.path1 == "value0" + assert config.path1 is None assert config.path2 == "value0/value2" diff --git a/test/unit/config/test_path_resolves_to.py b/test/unit/config/test_path_resolves_to.py index 97953b3d0787..d7e7dd0918d6 100644 --- a/test/unit/config/test_path_resolves_to.py +++ b/test/unit/config/test_path_resolves_to.py @@ -132,21 +132,21 @@ def test_kwargs_relative_path_old_prefix_for_other_option(mock_init): def test_kwargs_relative_path_old_prefix_empty_after_strip(mock_init): # Expect: use value from kwargs, strip old prefix, then resolve - new_path1 = "old-config" + new_path1 = "old-config/foo" config = BaseAppConfiguration(path1=new_path1) - assert config.path1 == "my-config/" # stripped of old prefix, then resolved + assert config.path1 == "my-config/foo" # stripped of old prefix, then resolved assert config.path2 == "my-data/my-data-files" # stripped of old prefix, then resolved assert config.path3 == "my-other-files" # no change def test_kwargs_set_to_null(mock_init): - # Expected: allow overriding with null, then resolve + # Expected: allow overriding with null # This is not a common scenario, but it does happen: one example is # `job_config` set to `None` when testing config = BaseAppConfiguration(path1=None) - assert config.path1 == "my-config" # resolved + assert config.path1 is None assert config.path2 == "my-data/my-data-files" # resolved assert config.path3 == "my-other-files" # no change