From 2e7a0720e7083a319ff97d898a1cd8f27c448d82 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 28 Oct 2019 15:58:40 -0400 Subject: [PATCH 01/13] Structured access to Galaxy internals for ITs. --- lib/galaxy/tool_util/parser/xml.py | 10 +++++- lib/galaxy/tools/evaluation.py | 25 +++++++++++++-- .../tools/environment_variables.xml | 32 +++++++++++++++++-- test/unit/tool_util/test_parsing.py | 10 ++++++ .../interactive/interactivetool_askomics.xml | 11 ++----- .../interactivetool_jupyter_notebook.xml | 15 +++------ 6 files changed, 76 insertions(+), 27 deletions(-) diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index ecfaf88cce1..b453f1c82ca 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -138,9 +138,17 @@ class XmlToolSource(ToolSource): environment_variables = [] for environment_variable_el in environment_variables_el.findall("environment_variable"): + template = environment_variable_el.text + inject = environment_variable_el.get("inject") + if inject: + assert not template, "Cannot specify inject and environment variable template." + assert inject in ["api_key"] + if template: + assert not inject, "Cannot specify inject and environment variable template." definition = { "name": environment_variable_el.get("name"), - "template": environment_variable_el.text, + "template": template, + "inject": inject, "strip": string_as_bool(environment_variable_el.get("strip", False)), } environment_variables.append( diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 028238b9ee4..1cfbfab51e9 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -70,7 +70,7 @@ class ToolEvaluator(object): incoming = self.tool.params_from_strings(incoming, self.app) # Full parameter validation - request_context = WorkRequestContext(app=self.app, user=job.history and job.history.user, history=job.history) + request_context = WorkRequestContext(app=self.app, user=self._user, history=self._history) def validate_inputs(input, value, context, **kwargs): value = input.from_json(value, request_context, context) @@ -134,7 +134,10 @@ class ToolEvaluator(object): param_dict["input"] = input param_dict['__datatypes_config__'] = param_dict['GALAXY_DATATYPES_CONF_FILE'] = os.path.join(job_working_directory, 'registry.xml') - + if self._history: + param_dict['__history_id'] = self.app.security.encode_id(self._history.id) + # TODO: Should be overridable per destination (fetch from compute_environment?) + param_dict['__galaxy_url'] = self.app.config.galaxy_infrastructure_url param_dict.update(self.tool.template_macro_params) # All parameters go into the param_dict param_dict.update(incoming) @@ -543,9 +546,16 @@ class ToolEvaluator(object): directory = self.local_working_directory environment_variable = environment_variable_def.copy() environment_variable_template = environment_variable_def["template"] + inject = environment_variable_def.get("inject") + if inject == "api_key": + from galaxy.managers import api_keys + environment_variable_template = api_keys.ApiKeyManager(self.app).get_or_create_api_key(self._user) + is_template = False + else: + is_template = True fd, config_filename = tempfile.mkstemp(dir=directory) os.close(fd) - self.__write_workdir_file(config_filename, environment_variable_template, param_dict, strip=environment_variable_def.get("strip", False)) + self.__write_workdir_file(config_filename, environment_variable_template, param_dict, is_template=is_template, strip=environment_variable_def.get("strip", False)) config_file_basename = os.path.basename(config_filename) # environment setup in job file template happens before `cd $working_directory` environment_variable["value"] = '`cat "$_GALAXY_JOB_DIR/%s"`' % config_file_basename @@ -628,3 +638,12 @@ class ToolEvaluator(object): compat. """ return self.compute_environment.sep().join(args) + + @property + def _history(self): + return self.job.history + + @property + def _user(self): + history = self._history + return history and history.user diff --git a/test/functional/tools/environment_variables.xml b/test/functional/tools/environment_variables.xml index 5834bdbda17..ec64e0b2998 100644 --- a/test/functional/tools/environment_variables.xml +++ b/test/functional/tools/environment_variables.xml @@ -7,17 +7,28 @@ ISTHREE #else# NOTTHREE #end if# + + + $__history_id + $__galaxy_url - echo "\$INTVAR" > $out_file1; - echo "\$FORTEST" >> $out_file1; - echo "\$IFTEST" >> $out_file1; + echo "\$INTVAR" > '$out_file1'; + echo "\$FORTEST" >> '$out_file1'; + echo "\$IFTEST" >> '$out_file1'; + echo "\$GX_API" > '$out_file_api_key'; + echo "\$GX_URL" > '$out_file_galaxy_url'; + echo "\$GX_HISTORY_ID" >'$out_file_history_id'; + + + @@ -29,6 +40,21 @@ NOTTHREE + + + + + + + + + + + + + + + diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index 434e631ecf4..da646d19894 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -618,6 +618,16 @@ class CollectionOutputYamlTestCase(BaseLoaderTestCase): assert len(output_collections) == 1 +class EnvironmentVariablesTestCase(BaseLoaderTestCase): + source_file_name = os.path.join(galaxy_directory(), "test/functional/tools/environment_variables.xml") + source_contents = None + + def test_tests(self): + tests_dict = self._tool_source.parse_tests_to_dict() + tests = tests_dict["tests"] + assert len(tests) == 1 + + class ExpectationsTestCase(BaseLoaderTestCase): source_file_name = os.path.join(galaxy_directory(), "test/functional/tools/detect_errors.xml") source_contents = None diff --git a/tools/interactive/interactivetool_askomics.xml b/tools/interactive/interactivetool_askomics.xml index c1ce9d3bd11..7f997b54e63 100644 --- a/tools/interactive/interactivetool_askomics.xml +++ b/tools/interactive/interactivetool_askomics.xml @@ -10,15 +10,8 @@ - ${__app__.config.galaxy_infrastructure_url} - - #if $__user__: - #for $api_key in $__user__.api_keys: - ${api_key.key} - #break - #end for - #end if - + $__galaxy_url + - ${__app__.security.encode_id($jupyter_notebook.history_id)} - ${__app__.config.galaxy_infrastructure_url} + $__history_id + $__galaxy_url 8080 - ${__app__.config.galaxy_infrastructure_url} - - #if $__user__: - #for $api_key in $__user__.api_keys: - ${api_key.key} - #break - #end for - #end if - + $__galaxy_url + Date: Mon, 28 Oct 2019 16:05:32 -0400 Subject: [PATCH 02/13] Allow per-destination overrride of galaxy_url for tools. The proper interaction between tools and jobs for determining Galaxy's URL from the destination of the tool. --- lib/galaxy/jobs/__init__.py | 7 +++++++ lib/galaxy/jobs/runners/pulsar.py | 3 +++ lib/galaxy/tools/evaluation.py | 3 +-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index e9816ce9be3..2c101828610 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -2401,6 +2401,10 @@ class ComputeEnvironment(object): def tmp_directory(self): """Temp directory of target job - none if HOME should not be set.""" + @abstractmethod + def galaxy_url(self): + """URL to access Galaxy API from for this compute environment.""" + class SimpleComputeEnvironment(object): @@ -2455,6 +2459,9 @@ class SharedComputeEnvironment(SimpleComputeEnvironment): def tmp_directory(self): return self.job_wrapper.tmp_directory() + def galaxy_url(self): + return self.job_wrapper.get_destination_configuration("galaxy_infrastructure_url") + class NoopQueue(object): """ diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py index f1ce2f2cf18..5dccf8352e1 100644 --- a/lib/galaxy/jobs/runners/pulsar.py +++ b/lib/galaxy/jobs/runners/pulsar.py @@ -982,6 +982,9 @@ class PulsarComputeEnvironment(ComputeEnvironment): # meantime. return None + def galaxy_url(self): + return self.job_wrapper.get_destination_configuration("galaxy_infrastructure_url") + class UnsupportedPulsarException(Exception): diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 1cfbfab51e9..008f33ef444 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -136,8 +136,7 @@ class ToolEvaluator(object): param_dict['__datatypes_config__'] = param_dict['GALAXY_DATATYPES_CONF_FILE'] = os.path.join(job_working_directory, 'registry.xml') if self._history: param_dict['__history_id'] = self.app.security.encode_id(self._history.id) - # TODO: Should be overridable per destination (fetch from compute_environment?) - param_dict['__galaxy_url'] = self.app.config.galaxy_infrastructure_url + param_dict['__galaxy_url'] = self.compute_environment.galaxy_url() param_dict.update(self.tool.template_macro_params) # All parameters go into the param_dict param_dict.update(incoming) From 4ccef702bea18bc88998be5dd225daa6ab216017 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 29 Oct 2019 08:29:02 -0400 Subject: [PATCH 03/13] Cleanup structured IT internals access commit. - Unit test fixes and added tests. - Change __history_id to __history_id__ per comment from @bgruening --- lib/galaxy/tools/evaluation.py | 4 ++-- test/functional/tools/environment_variables.xml | 4 ++-- test/unit/tools/test_evaluation.py | 17 +++++++++++++++++ tools/interactive/interactivetool_askomics.xml | 2 +- .../interactivetool_jupyter_notebook.xml | 6 +++--- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 008f33ef444..dc7b7bf6cc1 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -135,8 +135,8 @@ class ToolEvaluator(object): param_dict["input"] = input param_dict['__datatypes_config__'] = param_dict['GALAXY_DATATYPES_CONF_FILE'] = os.path.join(job_working_directory, 'registry.xml') if self._history: - param_dict['__history_id'] = self.app.security.encode_id(self._history.id) - param_dict['__galaxy_url'] = self.compute_environment.galaxy_url() + param_dict['__history_id__'] = self.app.security.encode_id(self._history.id) + param_dict['__galaxy_url__'] = self.compute_environment.galaxy_url() param_dict.update(self.tool.template_macro_params) # All parameters go into the param_dict param_dict.update(incoming) diff --git a/test/functional/tools/environment_variables.xml b/test/functional/tools/environment_variables.xml index ec64e0b2998..83982d9b125 100644 --- a/test/functional/tools/environment_variables.xml +++ b/test/functional/tools/environment_variables.xml @@ -10,8 +10,8 @@ NOTTHREE - $__history_id - $__galaxy_url + $__history_id__ + $__galaxy_url__ echo "\$INTVAR" > '$out_file1'; diff --git a/test/unit/tools/test_evaluation.py b/test/unit/tools/test_evaluation.py index 37243fbfdbf..ff94bffbf0a 100644 --- a/test/unit/tools/test_evaluation.py +++ b/test/unit/tools/test_evaluation.py @@ -33,6 +33,7 @@ from ..tools_support import UsesApp # To Test: # - param_file handling. TEST_TOOL_DIRECTORY = "/path/to/the/tool" +TEST_GALAXY_URL = "http://mycool.galaxyproject.org:8456" class ToolEvaluatorTestCase(TestCase, UsesApp): @@ -42,6 +43,7 @@ class ToolEvaluatorTestCase(TestCase, UsesApp): self.tool = MockTool(self.app) self.job = Job() self.job.history = History() + self.job.history.id = 42 self.job.parameters = [JobParameter(name="thresh", value="4")] self.evaluator = ToolEvaluator(self.app, self.tool, self.job, self.test_directory) @@ -65,6 +67,18 @@ class ToolEvaluatorTestCase(TestCase, UsesApp): command_line, extra_filenames, _ = self.evaluator.build() self.assertEqual(command_line, "prog1 4 5") + def test_eval_galaxy_url(self): + self.tool._command_line = "prog1 $__galaxy_url__" + self._set_compute_environment() + command_line, extra_filenames, _ = self.evaluator.build() + self.assertEqual(command_line, "prog1 %s" % TEST_GALAXY_URL) + + def test_eval_history_id(self): + self.tool._command_line = "prog1 '$__history_id__'" + self._set_compute_environment() + command_line, extra_filenames, _ = self.evaluator.build() + self.assertEqual(command_line, "prog1 '%s'" % self.app.security.encode_id(42)) + def test_conditional_evaluation(self): select_xml = XML('''''') parameter = SelectToolParameter(self.tool, select_xml) @@ -261,6 +275,9 @@ class TestComputeEnvironment(SimpleComputeEnvironment): def tool_directory(self): return TEST_TOOL_DIRECTORY + def galaxy_url(self): + return TEST_GALAXY_URL + class MockTool(object): diff --git a/tools/interactive/interactivetool_askomics.xml b/tools/interactive/interactivetool_askomics.xml index 7f997b54e63..b5f0b4d56a4 100644 --- a/tools/interactive/interactivetool_askomics.xml +++ b/tools/interactive/interactivetool_askomics.xml @@ -10,7 +10,7 @@ - $__galaxy_url + $__galaxy_url__ - $__history_id - $__galaxy_url + $__history_id__ + $__galaxy_url__ 8080 - $__galaxy_url + $__galaxy_url__ Date: Tue, 29 Oct 2019 08:51:48 -0400 Subject: [PATCH 04/13] Update galaxy.xsd for injecting API keys. --- lib/galaxy/tool_util/xsd/galaxy.xsd | 35 +++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index a5d6adf6779..bbc9c4a9c2d 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -4670,6 +4670,21 @@ variable instead of shell variable. ``` +### inject + +The Galaxy user's API key can be injected into an environment variable by setting ``inject`` +attribute to ``api_key`` (e.g. ``inject="api_key"``). + +```xml + + + +``` + +The framework allows setting this via environment variable and not via templating variables +in order to discourage setting the actual values of these keys as command line arguments. +On shared systems this provides some security by preventing a simple process listing command +from exposing keys. ]]> @@ -4680,15 +4695,27 @@ variable instead of shell variable. define. + + + Special variable to inject into the environment variable. Currently 'api_key' is the only option and will cause the user's API key to be injected via this environment variable. + + - - Whether to strip leading and trailing whitespace from the calculated value before exporting the environment variable. - + + Whether to strip leading and trailing whitespace from the calculated value before exporting the environment variable. + - + + + + + + + + Date: Tue, 29 Oct 2019 09:34:49 -0400 Subject: [PATCH 05/13] Fix id convention for test interactive tools. --- test/functional/tools/interactivetool_simple.xml | 2 +- test/functional/tools/interactivetool_two_entry_points.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/tools/interactivetool_simple.xml b/test/functional/tools/interactivetool_simple.xml index 54820286ef4..63e4c4b410e 100644 --- a/test/functional/tools/interactivetool_simple.xml +++ b/test/functional/tools/interactivetool_simple.xml @@ -1,4 +1,4 @@ - + galaxy/test-http-example:0.1 diff --git a/test/functional/tools/interactivetool_two_entry_points.xml b/test/functional/tools/interactivetool_two_entry_points.xml index 8693360840a..ffb59974df8 100644 --- a/test/functional/tools/interactivetool_two_entry_points.xml +++ b/test/functional/tools/interactivetool_two_entry_points.xml @@ -1,4 +1,4 @@ - + galaxy/test-http-example:0.1 From 8807a8f39f857f48f7ce953bf1022e9d01bdb4d6 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 30 Oct 2019 10:02:44 -0400 Subject: [PATCH 06/13] Separate new inject tests into new tool. --- .../tools/environment_variables.xml | 32 ++------------ .../tools/environment_variables_inject.xml | 42 +++++++++++++++++++ test/functional/tools/samples_tool_conf.xml | 1 + 3 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 test/functional/tools/environment_variables_inject.xml diff --git a/test/functional/tools/environment_variables.xml b/test/functional/tools/environment_variables.xml index 83982d9b125..5834bdbda17 100644 --- a/test/functional/tools/environment_variables.xml +++ b/test/functional/tools/environment_variables.xml @@ -7,28 +7,17 @@ ISTHREE #else# NOTTHREE #end if# - - - $__history_id__ - $__galaxy_url__ - echo "\$INTVAR" > '$out_file1'; - echo "\$FORTEST" >> '$out_file1'; - echo "\$IFTEST" >> '$out_file1'; - echo "\$GX_API" > '$out_file_api_key'; - echo "\$GX_URL" > '$out_file_galaxy_url'; - echo "\$GX_HISTORY_ID" >'$out_file_history_id'; + echo "\$INTVAR" > $out_file1; + echo "\$FORTEST" >> $out_file1; + echo "\$IFTEST" >> $out_file1; - - - @@ -40,21 +29,6 @@ NOTTHREE - - - - - - - - - - - - - - - diff --git a/test/functional/tools/environment_variables_inject.xml b/test/functional/tools/environment_variables_inject.xml new file mode 100644 index 00000000000..7b8cb5b6955 --- /dev/null +++ b/test/functional/tools/environment_variables_inject.xml @@ -0,0 +1,42 @@ + + + + + $__history_id__ + $__galaxy_url__ + + + echo "\$GX_API" > '$out_file_api_key'; + echo "\$GX_URL" > '$out_file_galaxy_url'; + echo "\$GX_HISTORY_ID" > '$out_file_history_id' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index a12e997bce9..a867645e74f 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -8,6 +8,7 @@ + From 58f2382ad0cf070ee2c04119e4fc4c13cdb99192 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 6 Nov 2019 06:44:14 -0500 Subject: [PATCH 07/13] Allow anonymous users to use tools with injected API keys. https://github.com/galaxyproject/galaxy/pull/8897/files#r342997635 --- lib/galaxy/tools/evaluation.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index dc7b7bf6cc1..c84cbf19f65 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -547,8 +547,11 @@ class ToolEvaluator(object): environment_variable_template = environment_variable_def["template"] inject = environment_variable_def.get("inject") if inject == "api_key": - from galaxy.managers import api_keys - environment_variable_template = api_keys.ApiKeyManager(self.app).get_or_create_api_key(self._user) + if self._user: + from galaxy.managers import api_keys + environment_variable_template = api_keys.ApiKeyManager(self.app).get_or_create_api_key(self._user) + else: + environment_variable_template = "" is_template = False else: is_template = True From 61b9aee3ff687184be7e66b63d41f32e31ec188d Mon Sep 17 00:00:00 2001 From: Bjoern Gruening Date: Wed, 6 Nov 2019 22:17:28 +0100 Subject: [PATCH 08/13] check_for_entry_points() more agressively in the condor job runner --- lib/galaxy/jobs/runners/condor.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/jobs/runners/condor.py b/lib/galaxy/jobs/runners/condor.py index 2ddb4030bc3..fdf6ed720ad 100644 --- a/lib/galaxy/jobs/runners/condor.py +++ b/lib/galaxy/jobs/runners/condor.py @@ -177,6 +177,9 @@ class CondorJobRunner(AsynchronousJobRunner): galaxy_id_tag = cjs.job_wrapper.get_id_tag() try: if os.stat(cjs.user_log).st_size == cjs.user_log_size: + if cjs.job_wrapper.tool.tool_type == 'interactive': + # If running, check for entry points... + cjs.job_wrapper.check_for_entry_points() new_watched.append(cjs) continue s1, s4, s7, s5, s9, log_size = summarize_condor_log(cjs.user_log, job_id) @@ -192,10 +195,6 @@ class CondorJobRunner(AsynchronousJobRunner): self.work_queue.put((self.fail_job, cjs)) continue - if job_running: - # If running, check for entry points... - cjs.job_wrapper.check_for_entry_points() - if job_running and not cjs.running: log.debug("(%s/%s) job is now running" % (galaxy_id_tag, job_id)) cjs.job_wrapper.change_state(model.Job.states.RUNNING) From ac1768c8d8ffd05f9f3c0116a51669b69a22e4ef Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 7 Nov 2019 20:48:36 +0100 Subject: [PATCH 09/13] Fix reading loc file samples This must've been broken for some time. Fixes https://github.com/galaxyproject/galaxy/issues/8534 --- lib/galaxy/tools/data/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tools/data/__init__.py b/lib/galaxy/tools/data/__init__.py index 35224d0b537..606046df8d2 100644 --- a/lib/galaxy/tools/data/__init__.py +++ b/lib/galaxy/tools/data/__init__.py @@ -382,21 +382,21 @@ class TabularToolDataTable(ToolDataTable, Dictifiable): filename = os.path.join(tool_data_path, filename) if self.tool_data_path_files.exists(filename): found = True - elif self.tool_data_path_files.exists("%s.sample" % filename) and not from_shed_config: - log.info("Could not find tool data %s, reading sample" % filename) - filename = "%s.sample" % filename - found = True else: # Since the path attribute can include a hard-coded path to a specific directory # (e.g., ) which may not be the same value # as self.tool_data_path, we'll parse the path to get the filename and see if it is # in self.tool_data_path. file_path, file_name = os.path.split(filename) - if file_path and file_path != self.tool_data_path: + if file_path != self.tool_data_path: corrected_filename = os.path.join(self.tool_data_path, file_name) if self.tool_data_path_files.exists(corrected_filename): filename = corrected_filename found = True + elif not from_shed_config and self.tool_data_path_files.exists("%s.sample" % corrected_filename): + log.info("Could not find tool data %s, reading sample" % corrected_filename) + filename = "%s.sample" % corrected_filename + found = True errors = [] if found: From 695d94bf8c47fbcce05fb224a36a9f3aff1568b4 Mon Sep 17 00:00:00 2001 From: Bjoern Gruening Date: Thu, 7 Nov 2019 23:43:00 +0100 Subject: [PATCH 10/13] adopt to Marius suggestion --- lib/galaxy/jobs/runners/condor.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/jobs/runners/condor.py b/lib/galaxy/jobs/runners/condor.py index fdf6ed720ad..2a887bd87ab 100644 --- a/lib/galaxy/jobs/runners/condor.py +++ b/lib/galaxy/jobs/runners/condor.py @@ -176,10 +176,7 @@ class CondorJobRunner(AsynchronousJobRunner): job_id = cjs.job_id galaxy_id_tag = cjs.job_wrapper.get_id_tag() try: - if os.stat(cjs.user_log).st_size == cjs.user_log_size: - if cjs.job_wrapper.tool.tool_type == 'interactive': - # If running, check for entry points... - cjs.job_wrapper.check_for_entry_points() + if cjs.job_wrapper.tool.tool_type != 'interactive' and os.stat(cjs.user_log).st_size == cjs.user_log_size: new_watched.append(cjs) continue s1, s4, s7, s5, s9, log_size = summarize_condor_log(cjs.user_log, job_id) @@ -195,6 +192,10 @@ class CondorJobRunner(AsynchronousJobRunner): self.work_queue.put((self.fail_job, cjs)) continue + if job_running: + # If running, check for entry points... + cjs.job_wrapper.check_for_entry_points() + if job_running and not cjs.running: log.debug("(%s/%s) job is now running" % (galaxy_id_tag, job_id)) cjs.job_wrapper.change_state(model.Job.states.RUNNING) From 81532d9c647e3cda4c7410c3071640cac82998e6 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 8 Nov 2019 11:26:27 +0100 Subject: [PATCH 11/13] Validate tag strings as well as list of tags I think it'd be better if we consistently used lists for tags, but it seems too late in 19.09 to make possibly bigger changes here. Fixes https://github.com/galaxyproject/galaxy/issues/8851 broken in https://github.com/galaxyproject/galaxy/pull/8530. --- lib/galaxy/managers/library_datasets.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py index 2f844e57c15..f93b23577c8 100644 --- a/lib/galaxy/managers/library_datasets.py +++ b/lib/galaxy/managers/library_datasets.py @@ -139,7 +139,10 @@ class LibraryDatasetsManager(datasets.DatasetAssociationManager): val = validation.validate_and_sanitize_basestring(key, val) validated_payload[key] = val if key in ('tags'): - val = validation.validate_and_sanitize_basestring_list(key, val) + if isinstance(val, list): + val = validation.validate_and_sanitize_basestring_list(key, val) + else: + val = validation.validate_and_sanitize_basestring(key, val) validated_payload[key] = val return validated_payload From 2423a5b2dcba77582c25a554d6a75d79c726a42d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 8 Nov 2019 13:18:33 +0100 Subject: [PATCH 12/13] Fix send_local_control_task argument passing `kwargs` used to be the second argument, but it is now being interpreted as `get_response`. Since there's no response coming it'll hang until a timeout occurs. Fixes ``` galaxy.queue_worker: ERROR: Error waiting for task: '{'kwargs': {}, 'task': 'recalculate_user_disk_usage'}' sent with routing key 'control.main@613c834ba62c' Traceback (most recent call last): File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/virtual/base.py", line 979, in drain_events get(self._deliver, timeout=timeout) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/utils/scheduling.py", line 56, in get return self.fun(resource, callback, **kwargs) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/virtual/base.py", line 1017, in _drain_channel return channel.drain_events(callback=callback, timeout=timeout) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/virtual/base.py", line 761, in drain_events return self._poll(self.cycle, callback, timeout=timeout) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/virtual/base.py", line 402, in _poll return cycle.get(callback) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/utils/scheduling.py", line 56, in get return self.fun(resource, callback, **kwargs) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/virtual/base.py", line 405, in _get_and_deliver message = self._get(queue) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/sqlalchemy/__init__.py", line 109, in _get raise Empty() queue.Empty During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/galaxy/lib/galaxy/queue_worker.py", line 130, in send_task self.connection.drain_events(timeout=timeout) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/connection.py", line 321, in drain_events return self.transport.drain_events(self.connection, **kwargs) File "/galaxy_venv3/lib/python3.5/site-packages/kombu/transport/virtual/base.py", line 982, in drain_events raise socket.timeout() socket.timeout ``` That occasionally occurs in the selenium tests. --- lib/galaxy/webapps/galaxy/controllers/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 99db8aff2c7..d5bfc953833 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -225,7 +225,7 @@ class User(BaseUIController, UsesFormDefinitionsMixin, CreatesApiKeysMixin): # while sometimes, so we don't want to block on logout. send_local_control_task(trans.app, "recalculate_user_disk_usage", - {"user_id": trans.security.encode_id(trans.user.id)}) + kwargs={"user_id": trans.security.encode_id(trans.user.id)}) # Since logging an event requires a session, we'll log prior to ending the session trans.log_event("User logged out") trans.handle_user_logout(logout_all=logout_all) From 3013895f785bcff07686a22061b32a1da4c7da7c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 8 Nov 2019 14:12:31 +0100 Subject: [PATCH 13/13] Use listify to convert tag string to list tag should be a list in `_set_from_dict`. Thanks @nsoranzo. --- lib/galaxy/managers/library_datasets.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/galaxy/managers/library_datasets.py b/lib/galaxy/managers/library_datasets.py index f93b23577c8..3b52b6a6227 100644 --- a/lib/galaxy/managers/library_datasets.py +++ b/lib/galaxy/managers/library_datasets.py @@ -139,10 +139,7 @@ class LibraryDatasetsManager(datasets.DatasetAssociationManager): val = validation.validate_and_sanitize_basestring(key, val) validated_payload[key] = val if key in ('tags'): - if isinstance(val, list): - val = validation.validate_and_sanitize_basestring_list(key, val) - else: - val = validation.validate_and_sanitize_basestring(key, val) + val = validation.validate_and_sanitize_basestring_list(key, util.listify(val)) validated_payload[key] = val return validated_payload