Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.2] Partial backport of #19331 #19342

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/galaxy/config/__init__.py
Original file line number Diff line number Diff line change
@@ -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!
@@ -538,7 +538,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):
@@ -557,6 +557,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 = []
2 changes: 2 additions & 0 deletions lib/galaxy/jobs/__init__.py
Original file line number Diff line number Diff line change
@@ -384,6 +384,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)
7 changes: 5 additions & 2 deletions lib/galaxy/tool_util/linters/datatypes.py
Original file line number Diff line number Diff line change
@@ -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()
25 changes: 17 additions & 8 deletions lib/galaxy/util/__init__.py
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions test/unit/config/test_path_graph.py
Original file line number Diff line number Diff line change
@@ -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,13 +155,13 @@ 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))
monkeypatch.setattr(BaseAppConfiguration, "_load_schema", lambda a: AppSchema(Path("no path"), "_"))

config = BaseAppConfiguration()
assert config.path0 == "value0"
assert config.path1 == "value0"
assert config.path1 is None
assert config.path2 == "value0/value2"
8 changes: 4 additions & 4 deletions test/unit/config/test_path_resolves_to.py
Original file line number Diff line number Diff line change
@@ -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