From 7630ed54277a7fa453f3a86b7d202e156de92b16 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 2 Dec 2021 16:11:23 +0000 Subject: [PATCH 1/5] Fix `clean-cwl-conformance-tests` missing from `make` menu --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 04da9cb3cd7..d807be0af3e 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,7 @@ $(CWL_TARGETS): generate-cwl-conformance-tests: $(CWL_TARGETS) ## Initialise CWL conformance tests -clean-cwl-conformance-tests: # Clean CWL conformance tests +clean-cwl-conformance-tests: ## Clean CWL conformance tests for f in $(CWL_TARGETS); do \ if [ $$(basename "$$f") = conformance_tests.yaml ]; then \ rm -rf $$(dirname "$$f"); \ From a0cf05ae7165d93db500a1088e3284366651af97 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 2 Dec 2021 16:12:43 +0000 Subject: [PATCH 2/5] Don't hide exception --- lib/galaxy/app_unittest_utils/tools_support.py | 9 +++------ test/unit/app/tools/test_toolbox.py | 4 +++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/app_unittest_utils/tools_support.py b/lib/galaxy/app_unittest_utils/tools_support.py index 5dc91ff30b2..75394b3f3f4 100644 --- a/lib/galaxy/app_unittest_utils/tools_support.py +++ b/lib/galaxy/app_unittest_utils/tools_support.py @@ -110,12 +110,9 @@ class UsesTools(UsesApp): def __setup_tool(self): tool_source = get_tool_source(self.tool_file) - try: - self.tool = create_tool_from_source(self.app, tool_source, config_file=self.tool_file) - self.tool.assert_finalized() - except Exception: - self.tool = None - if getattr(self, "tool_action", None) and self.tool: + self.tool = create_tool_from_source(self.app, tool_source, config_file=self.tool_file) + self.tool.assert_finalized() + if getattr(self, "tool_action", None): self.tool.tool_action = self.tool_action return self.tool diff --git a/test/unit/app/tools/test_toolbox.py b/test/unit/app/tools/test_toolbox.py index f1d767a4490..9af7f9f9bb5 100644 --- a/test/unit/app/tools/test_toolbox.py +++ b/test/unit/app/tools/test_toolbox.py @@ -281,7 +281,9 @@ class ToolBoxTestCase(BaseToolBoxTestCase): def test_enforce_tool_profile(self): self._init_tool(filename="old_tool.xml", version="1.0", profile="17.01", tool_id="test_old_tool_profile") - self._init_tool(filename="new_tool.xml", version="2.0", profile="27.01", tool_id="test_new_tool_profile") + with self.assertRaisesRegex(Exception, r"The tool \[test_new_tool_profile\] targets version 37\.01 of Galaxy"): + # This will write the file but fail to load the tool + self._init_tool(filename="new_tool.xml", version="2.0", profile="37.01", tool_id="test_new_tool_profile") self._add_config("""""") toolbox = self.toolbox assert toolbox.get_tool("test_old_tool_profile") is not None From ee061229b814d02f80c830618d6ce876fdab2647 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 2 Dec 2021 17:11:42 +0000 Subject: [PATCH 3/5] Initialise UsesTools only once --- lib/galaxy/app_unittest_utils/tools_support.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/app_unittest_utils/tools_support.py b/lib/galaxy/app_unittest_utils/tools_support.py index 75394b3f3f4..349bc291350 100644 --- a/lib/galaxy/app_unittest_utils/tools_support.py +++ b/lib/galaxy/app_unittest_utils/tools_support.py @@ -94,15 +94,14 @@ class UsesTools(UsesApp): self.__write_tool(extra_file_contents, path=os.path.join(self.test_directory, extra_file_path)) else: self.tool_file = tool_path - self._init_app_for_tools() return self.__setup_tool() def _init_tool_for_path(self, tool_file): - self._init_app_for_tools() self.tool_file = tool_file return self.__setup_tool() - def _init_app_for_tools(self): + def setup_app(self): + super().setup_app() self.app.config.drmaa_external_runjob_script = "" self.app.config.tool_secret = "testsecret" self.app.config.track_jobs_in_database = False From 5d3648af92fc17d93e1ce7259f752b3dcfcd63fa Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 2 Dec 2021 18:55:02 +0000 Subject: [PATCH 4/5] Better error message for unknown `ShellJobRunner` shell plugin Instead of raising: ``` File "/srv/galaxy/lib/galaxy/jobs/runners/util/cli/__init__.py", line 78, in get_shell_plugin self.active_cli_shells[requested_shell_settings] = self.cli_shells[shell_plugin](**shell_params) KeyError: 'ZecureShell' ``` raise: ``` File "/srv/galaxy/lib/galaxy/jobs/runners/util/cli/__init__.py", line 80, in get_shell_plugin raise ValueError(f"Unknown shell_plugin [{shell_plugin}], available plugins are {list(self.cli_shells.keys())}") ValueError: Unknown shell_plugin [ZecureShell], available plugins are ['RemoteShell', 'SecureShell', 'GlobusSecureShell', 'ParamikoShell', 'LocalShell'] ``` --- lib/galaxy/jobs/runners/util/cli/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/jobs/runners/util/cli/__init__.py b/lib/galaxy/jobs/runners/util/cli/__init__.py index c63eb070984..c38bb878ff6 100644 --- a/lib/galaxy/jobs/runners/util/cli/__init__.py +++ b/lib/galaxy/jobs/runners/util/cli/__init__.py @@ -75,7 +75,10 @@ class CliInterface: shell_plugin = shell_params.get('plugin', DEFAULT_SHELL_PLUGIN) requested_shell_settings = json.dumps(shell_params, sort_keys=True) if requested_shell_settings not in self.active_cli_shells: - self.active_cli_shells[requested_shell_settings] = self.cli_shells[shell_plugin](**shell_params) + shell_plugin_class = self.cli_shells.get(shell_plugin) + if not shell_plugin_class: + raise ValueError(f"Unknown shell_plugin [{shell_plugin}], available plugins are {list(self.cli_shells.keys())}") + self.active_cli_shells[requested_shell_settings] = shell_plugin_class(**shell_params) return self.active_cli_shells[requested_shell_settings] def get_job_interface(self, job_params): From 6e59e2de5f8a5364eb3a474f1f2327f33aa29dcb Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 2 Dec 2021 18:59:51 +0000 Subject: [PATCH 5/5] Simplify code --- lib/galaxy/jobs/runners/util/cli/__init__.py | 8 +++----- lib/galaxy/jobs/runners/util/cli/job/__init__.py | 2 +- lib/galaxy/jobs/runners/util/cli/job/lsf.py | 5 ----- lib/galaxy/jobs/runners/util/cli/job/slurm.py | 5 ----- lib/galaxy/jobs/runners/util/cli/job/torque.py | 5 ----- 5 files changed, 4 insertions(+), 21 deletions(-) diff --git a/lib/galaxy/jobs/runners/util/cli/__init__.py b/lib/galaxy/jobs/runners/util/cli/__init__.py index c38bb878ff6..8aecd72f5c3 100644 --- a/lib/galaxy/jobs/runners/util/cli/__init__.py +++ b/lib/galaxy/jobs/runners/util/cli/__init__.py @@ -82,15 +82,13 @@ class CliInterface: return self.active_cli_shells[requested_shell_settings] def get_job_interface(self, job_params): - job_plugin = job_params.get('plugin', None) + job_plugin = job_params.get('plugin') if not job_plugin: raise ValueError(ERROR_MESSAGE_NO_JOB_PLUGIN) - job_plugin_class = self.cli_job_interfaces.get(job_plugin, None) + job_plugin_class = self.cli_job_interfaces.get(job_plugin) if not job_plugin_class: raise ValueError(ERROR_MESSAGE_NO_SUCH_JOB_PLUGIN % (job_plugin, list(self.cli_job_interfaces.keys()))) - job_interface = job_plugin_class(**job_params) - - return job_interface + return job_plugin_class(**job_params) def split_params(params): diff --git a/lib/galaxy/jobs/runners/util/cli/job/__init__.py b/lib/galaxy/jobs/runners/util/cli/job/__init__.py index ea5c862c95a..73d3f0b53e3 100644 --- a/lib/galaxy/jobs/runners/util/cli/job/__init__.py +++ b/lib/galaxy/jobs/runners/util/cli/job/__init__.py @@ -22,11 +22,11 @@ except ImportError: class BaseJobExec(metaclass=ABCMeta): - @abstractmethod def __init__(self, **params): """ Constructor for CLI job executor. """ + self.params = params.copy() def job_script_kwargs(self, ofile, efile, job_name): """ Return extra keyword argument for consumption by job script diff --git a/lib/galaxy/jobs/runners/util/cli/job/lsf.py b/lib/galaxy/jobs/runners/util/cli/job/lsf.py index a3ad27e123c..a1e871b732d 100644 --- a/lib/galaxy/jobs/runners/util/cli/job/lsf.py +++ b/lib/galaxy/jobs/runners/util/cli/job/lsf.py @@ -19,11 +19,6 @@ argmap = { class LSF(BaseJobExec): - def __init__(self, **params): - self.params = {} - for k, v in params.items(): - self.params[k] = v - def job_script_kwargs(self, ofile, efile, job_name): scriptargs = {'-o': ofile, '-e': efile, diff --git a/lib/galaxy/jobs/runners/util/cli/job/slurm.py b/lib/galaxy/jobs/runners/util/cli/job/slurm.py index d87479b513d..9ae10295ace 100644 --- a/lib/galaxy/jobs/runners/util/cli/job/slurm.py +++ b/lib/galaxy/jobs/runners/util/cli/job/slurm.py @@ -15,11 +15,6 @@ argmap = { class Slurm(BaseJobExec): - def __init__(self, **params): - self.params = {} - for k, v in params.items(): - self.params[k] = v - def job_script_kwargs(self, ofile, efile, job_name): scriptargs = {'-o': ofile, '-e': efile, diff --git a/lib/galaxy/jobs/runners/util/cli/job/torque.py b/lib/galaxy/jobs/runners/util/cli/job/torque.py index e369afab390..855519f8f8c 100644 --- a/lib/galaxy/jobs/runners/util/cli/job/torque.py +++ b/lib/galaxy/jobs/runners/util/cli/job/torque.py @@ -31,11 +31,6 @@ argmap = {'destination': '-q', class Torque(BaseJobExec): - def __init__(self, **params): - self.params = {} - for k, v in params.items(): - self.params[k] = v - def job_script_kwargs(self, ofile, efile, job_name): pbsargs = {'-o': ofile, '-e': efile,