From 48df4b910dff66578a00fa40aeb5aececdf99dcc Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 19 Jul 2021 15:17:59 -0400 Subject: [PATCH] Systematic handling of remotely required tool files. - Allow tools to define "pulsar file action"-like paths for files to include and exclude during remote job setup. - Package up a list of defaults for older tools. --- doc/schema_template.md | 3 + lib/galaxy/jobs/runners/pulsar.py | 3 +- lib/galaxy/tool_util/parser/__init__.py | 3 +- lib/galaxy/tool_util/parser/interface.py | 58 ++++++ lib/galaxy/tool_util/parser/xml.py | 22 +++ lib/galaxy/tool_util/xsd/galaxy.xsd | 72 ++++++++ lib/galaxy/tools/__init__.py | 21 +++ packages/app/requirements.txt | 2 +- test/unit/tool_util/test_required_files.py | 170 ++++++++++++++++++ .../tool_directories/r-tool-dir/my_script.R | 1 + .../r-tool-dir/other_script.R | 1 + 11 files changed, 353 insertions(+), 3 deletions(-) create mode 100644 test/unit/tool_util/test_required_files.py create mode 100644 test/unit/tool_util/tool_directories/r-tool-dir/my_script.R create mode 100644 test/unit/tool_util/tool_directories/r-tool-dir/other_script.R diff --git a/doc/schema_template.md b/doc/schema_template.md index 2f98d31a22d..a78b21180bf 100644 --- a/doc/schema_template.md +++ b/doc/schema_template.md @@ -26,6 +26,9 @@ $tag:tool|creator|organization://complexType[@name='Organization'] $tag:tool|requirements://complexType[@name='Requirements'] $tag:tool|requirements|requirement://complexType[@name='Requirement'] $tag:tool|requirements|container://complexType[@name='Container'] +$tag:tool|required_files://complexType[@name='RequiredFiles'] +$tag:tool|required_files|include://complexType[@name='RequiredFileInclude'] +$tag:tool|required_files|exclude://complexType[@name='RequiredFileExclude'] $tag:tool|code://complexType[@name='Code'] $tag:tool|stdio://complexType[@name='Stdio'] $tag:tool|stdio|exit_code://complexType[@name='ExitCode'] hide_attributes diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py index 5d7dfa19830..d84e3af48ee 100644 --- a/lib/galaxy/jobs/runners/pulsar.py +++ b/lib/galaxy/jobs/runners/pulsar.py @@ -395,7 +395,7 @@ class PulsarJobRunner(AsynchronousJobRunner): job_directory_path = tool_env.get("job_directory_path") if job_directory_path: config_files.append(job_directory_path) - + tool_directory_required_files = job_wrapper.tool.required_files client_job_description = ClientJobDescription( command_line=command_line, input_files=input_files, @@ -414,6 +414,7 @@ class PulsarJobRunner(AsynchronousJobRunner): job_directory_files=job_directory_files, container=None if not remote_container else remote_container.container_id, guest_ports=job_wrapper.guest_ports, + tool_directory_required_files=tool_directory_required_files, ) job_id = pulsar_submit_job(client, client_job_description, remote_job_config) log.info(f"Pulsar job submitted with job_id {job_id}") diff --git a/lib/galaxy/tool_util/parser/__init__.py b/lib/galaxy/tool_util/parser/__init__.py index 771fbaf3202..2a215042946 100644 --- a/lib/galaxy/tool_util/parser/__init__.py +++ b/lib/galaxy/tool_util/parser/__init__.py @@ -1,7 +1,7 @@ """ Package responsible for parsing tools from files/abstract tool sources. """ from .factory import get_input_source, get_tool_source, get_tool_source_from_representation -from .interface import ToolSource +from .interface import RequiredFiles, ToolSource from .output_objects import ( ToolOutputCollectionPart, ) @@ -10,6 +10,7 @@ __all__ = ( "get_input_source", "get_tool_source", "get_tool_source_from_representation", + "RequiredFiles", "ToolOutputCollectionPart", "ToolSource", ) diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index a30040a19b2..8cfead61887 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -1,9 +1,14 @@ +import fnmatch import os +import re from abc import ( ABCMeta, abstractmethod ) +from os.path import join +from typing import Dict, List, Optional +from galaxy.util.path import safe_walk from .util import _parse_name NOT_IMPLEMENTED_MESSAGE = "Galaxy tool format does not yet support this tool feature." @@ -167,6 +172,10 @@ class ToolSource(metaclass=ABCMeta): """ return False + def parse_required_files(self) -> Optional['RequiredFiles']: + """ Parse explicit RequiredFiles object or return None to let Galaxy decide implicit logic.""" + return None + @abstractmethod def parse_requirements_and_containers(self): """ Return pair of ToolRequirement and ContainerDescription lists. """ @@ -452,6 +461,55 @@ class TestCollectionDef: return inputs +class RequiredFiles: + + def __init__(self, includes: List[Dict], excludes: List[Dict], extend_default_excludes: bool): + self.includes = includes + self.excludes = excludes + self.extend_default_excludes = extend_default_excludes + + @staticmethod + def from_dict(as_dict): + extend_default_excludes: bool = as_dict.get("extend_default_excludes", True) + includes: List = as_dict.get("includes", []) + excludes: List = as_dict.get("excludes", []) + return RequiredFiles(includes, excludes, extend_default_excludes) + + def find_required_files(self, tool_directory: str) -> List[str]: + + def matches(ie_list: List, rel_path: str): + for ie_item in ie_list: + ie_item_path = ie_item["path"] + ie_item_type = ie_item.get("path_type", "literal") + if ie_item_type == "literal": + if rel_path == ie_item_path: + return True + elif ie_item_type == "prefix": + if rel_path.startswith(ie_item_path): + return True + elif ie_item_type == "glob": + if fnmatch.fnmatch(rel_path, ie_item_path): + return True + else: + if re.match(ie_item_path, rel_path) is not None: + return True + return False + + excludes = self.excludes + if self.extend_default_excludes: + excludes.append({"path": "tool-data", "path_type": "prefix"}) + excludes.append({"path": "test-data", "path_type": "prefix"}) + excludes.append({"path": ".hg", "path_type": "prefix"}) + + files: List[str] = [] + for (dirpath, _, filenames) in safe_walk(tool_directory): + for filename in filenames: + rel_path = join(dirpath, filename).replace(tool_directory + os.path.sep, '') + if matches(self.includes, rel_path) and not matches(self.excludes, rel_path): + files.append(rel_path) + return files + + class TestCollectionOutputDef: __test__ = False # Prevent pytest from discovering this class (issue #12071) diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 3af561ca65c..a3bde5e3f2d 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -3,6 +3,7 @@ import logging import re import uuid from math import isinf +from typing import Optional import packaging.version @@ -20,6 +21,7 @@ from .interface import ( InputSource, PageSource, PagesSource, + RequiredFiles, TestCollectionDef, TestCollectionOutputDef, ToolSource, @@ -261,6 +263,26 @@ class XmlToolSource(ToolSource): elem = self.root return string_as_bool(elem.get(attribute, default)) + def parse_required_files(self) -> Optional[RequiredFiles]: + required_files = self.root.find("required_files") + if required_files is None: + return None + + def parse_include_exclude_list(tag_name): + as_list = [] + for ref in required_files.findall(tag_name): + path = ref.get("path") + assert path is not None, f'"path" must be specified in {tag_name}' + path_type = ref.get("type", "literal") + as_list.append({"path": path, "path_type": path_type}) + return as_list + + as_dict = {} + as_dict["extend_default_excludes"] = self._get_attribute_as_bool("extend_default_excludes", True, elem=required_files) + as_dict["includes"] = parse_include_exclude_list("include") + as_dict["excludes"] = parse_include_exclude_list("exclude") + return RequiredFiles.from_dict(as_dict) + def parse_requirements_and_containers(self): return requirements.parse_requirements_from_xml(self.root) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 719962630b6..2ecbbf6eec5 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -89,6 +89,7 @@ A ``data_source`` tool contains a few more relevant attributes. + @@ -522,6 +523,77 @@ Describes an organization. Tries to stay close to [schema.org/Organization](http + + + + + + + + + + + Set this to `false` to override the default excludes for mercurial, reference, and test data. + + + + + + + How are files referenced in RequiredFileIncludes and RequiredFileExcludes. Paths are matched relative to the tool directory. `literal` must match the filename exactly. `prefix` will match paths based on their start. `glob` and `regex` use patterns to match files. + + + + + + + + + + + + Describe files to include when relocating tool directory for remote execution. + + + + Type of file reference `path` is. + + + + + Path to referenced files - this should be relative to the tool's directory (this is the file the tool is located in not the repository directory if these conflict). + + + + + + + Describe files to exclude when relocating tool directory for remote execution. + + + + Type of file reference `path` is. + + + + + Path to referenced files - this should be relative to the tool's directory (this is the file the tool is located in not the repository directory if these conflict). + + + + =0.14.7 gxformat2 Mako sqlitedict diff --git a/test/unit/tool_util/test_required_files.py b/test/unit/tool_util/test_required_files.py new file mode 100644 index 00000000000..f65ceca32e1 --- /dev/null +++ b/test/unit/tool_util/test_required_files.py @@ -0,0 +1,170 @@ +import os +from pathlib import Path + +from .test_parsing import BaseLoaderTestCase + +SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) +TEST_TOOL_DIRECTORIES = os.path.join(SCRIPT_DIRECTORY, "tool_directories") + + +TOOL_REQUIRED_FILES_XML_1 = """ + + foo + + + + + + + + + + +""" + + +TOOL_REQUIRED_FILES_XML_2 = """ + + foo + + + + + + + + + + + +""" + + +TOOL_REQUIRED_FILES_XML_3 = """ + + foo + + + + + + + + + + +""" + + +TOOL_REQUIRED_FILES_XML_4 = """ + + foo + + + + + + + + + + + +""" + + +TOOL_REQUIRED_FILES_XML_DISABLED_DEFAULT_EXCLUSIONS = """ + + foo + + + + + + + + + + +""" + + +class BaseRequiredFilesTestCase(BaseLoaderTestCase): + source_file_name = "required_files.xml" + + def _required_files(self, tool_directory: str): + tool_source = self._tool_source + required_files = tool_source.parse_required_files() + return required_files.find_required_files(tool_directory) + + +# directly include just one file that is there +class RequiredFiles1TestCase(BaseRequiredFilesTestCase): + source_contents = TOOL_REQUIRED_FILES_XML_1 + + def test_expected_files(self): + files = self._required_files(os.path.join(TEST_TOOL_DIRECTORIES, "r-tool-dir")) + assert len(files) == 1 + assert "my_script.R" in files + + +# include a glob and exclude a file in the glob +class RequiredFiles2TestCase(BaseRequiredFilesTestCase): + source_contents = TOOL_REQUIRED_FILES_XML_2 + + def test_expected_files(self): + files = self._required_files(os.path.join(TEST_TOOL_DIRECTORIES, "r-tool-dir")) + assert len(files) == 1 + assert "my_script.R" in files + + +# include a glob with multiple matches +class RequiredFiles3TestCase(BaseRequiredFilesTestCase): + source_contents = TOOL_REQUIRED_FILES_XML_3 + + def test_expected_files(self): + files = self._required_files(os.path.join(TEST_TOOL_DIRECTORIES, "r-tool-dir")) + assert len(files) == 2 + assert "my_script.R" in files + assert "other_script.R" in files + + +# include a file with regex and exclude with glob +class RequiredFiles4TestCase(BaseRequiredFilesTestCase): + source_contents = TOOL_REQUIRED_FILES_XML_4 + + def test_expected_files(self): + files = self._required_files(os.path.join(TEST_TOOL_DIRECTORIES, "r-tool-dir")) + assert len(files) == 1 + assert "my_script.R" in files + + +class HgExcludedByDefaultTestCase(BaseRequiredFilesTestCase): + source_contents = TOOL_REQUIRED_FILES_XML_3 + + def test_expected_files(self): + repo_dir = setup_dir_with_repo(self.temp_directory) + files = self._required_files(repo_dir) + assert len(files) == 1 + assert "my_script.R" in files + + +class HgExclusionDisabledTestCase(BaseRequiredFilesTestCase): + source_contents = TOOL_REQUIRED_FILES_XML_DISABLED_DEFAULT_EXCLUSIONS + + def test_expected_files(self): + repo_dir = setup_dir_with_repo(self.temp_directory) + files = self._required_files(repo_dir) + assert len(files) == 2 + assert "my_script.R" in files + assert ".hg/index.R" in files + + +def setup_dir_with_repo(tmp_dir): + repo = os.path.join(tmp_dir, "repo") + os.makedirs(repo) + hg = os.path.join(repo, ".hg") + os.makedirs(hg) + Path(hg, "index.R").touch() + Path(repo, "my_script.R").touch() + return repo diff --git a/test/unit/tool_util/tool_directories/r-tool-dir/my_script.R b/test/unit/tool_util/tool_directories/r-tool-dir/my_script.R new file mode 100644 index 00000000000..fee7cb0d403 --- /dev/null +++ b/test/unit/tool_util/tool_directories/r-tool-dir/my_script.R @@ -0,0 +1 @@ +my cool R script diff --git a/test/unit/tool_util/tool_directories/r-tool-dir/other_script.R b/test/unit/tool_util/tool_directories/r-tool-dir/other_script.R new file mode 100644 index 00000000000..6e7a2cc1241 --- /dev/null +++ b/test/unit/tool_util/tool_directories/r-tool-dir/other_script.R @@ -0,0 +1 @@ +another R script