From 352e751927cd6e8e521db383cbd21f50a51c1b74 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 29 Oct 2019 10:12:38 -0400 Subject: [PATCH 1/8] Revert hacking IPs in container monitor script. It breaks the Mac setup and seems just wrong, docker might not be on localhost. This needs to be made more specific in some way and should handle those socker commands not working on OS X. --- lib/galaxy_ext/container_monitor/monitor.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/galaxy_ext/container_monitor/monitor.py b/lib/galaxy_ext/container_monitor/monitor.py index e03cda18b9e..df2b13b8118 100644 --- a/lib/galaxy_ext/container_monitor/monitor.py +++ b/lib/galaxy_ext/container_monitor/monitor.py @@ -41,12 +41,8 @@ def main(): try: ports_raw = parse_ports(container_name, connection_configuration) if ports_raw is not None: - host_ip = socket.gethostbyname(socket.gethostname()) with open("container_runtime.json", "w") as f: - # Override the IPs ports = docker_util.parse_port_text(ports_raw) - for key in ports: - ports[key]['host'] = host_ip json.dump(ports, f) break else: From ff08885c2ea0215430c86b7377c1d1bf291e51f3 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 30 Oct 2019 11:03:20 -0400 Subject: [PATCH 2/8] Only re-map container monitor host if can find IP and have 0.0.0.0. --- lib/galaxy_ext/container_monitor/monitor.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/galaxy_ext/container_monitor/monitor.py b/lib/galaxy_ext/container_monitor/monitor.py index df2b13b8118..a64cb75d7d6 100644 --- a/lib/galaxy_ext/container_monitor/monitor.py +++ b/lib/galaxy_ext/container_monitor/monitor.py @@ -41,8 +41,16 @@ def main(): try: ports_raw = parse_ports(container_name, connection_configuration) if ports_raw is not None: + try: + host_ip = socket.gethostbyname(socket.gethostname()) + except Exception: + # doesn't work on OS X + host_ip = None with open("container_runtime.json", "w") as f: ports = docker_util.parse_port_text(ports_raw) + for key in ports: + if ports[key]['host'] == '0.0.0.0' and host_ip is not None: + ports[key]['host'] = host_ip json.dump(ports, f) break else: From 391381d3c40b16c7b9fdbadd9fa8f1f215cb7a59 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 30 Oct 2019 11:08:27 -0400 Subject: [PATCH 3/8] small doctest for docker port parsing --- lib/galaxy/tool_util/deps/docker_util.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/galaxy/tool_util/deps/docker_util.py b/lib/galaxy/tool_util/deps/docker_util.py index e5ef9cb7587..12432c9bbba 100644 --- a/lib/galaxy/tool_util/deps/docker_util.py +++ b/lib/galaxy/tool_util/deps/docker_util.py @@ -225,6 +225,12 @@ def _docker_prefix( def parse_port_text(port_text): + """ + + >>> slurm_ports = parse_port_text("8888/tcp -> 0.0.0.0:32769") + >>> slurm_ports[8888]['host'] + '0.0.0.0' + """ ports = None if port_text is not None: ports = {} From 1663c9e331ee2445ce86a2bca1a6cbd19dae64e1 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 30 Oct 2019 15:30:03 +0000 Subject: [PATCH 4/8] Check `host_ip` only once --- lib/galaxy_ext/container_monitor/monitor.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/galaxy_ext/container_monitor/monitor.py b/lib/galaxy_ext/container_monitor/monitor.py index a64cb75d7d6..b1e1447f1e0 100644 --- a/lib/galaxy_ext/container_monitor/monitor.py +++ b/lib/galaxy_ext/container_monitor/monitor.py @@ -48,9 +48,10 @@ def main(): host_ip = None with open("container_runtime.json", "w") as f: ports = docker_util.parse_port_text(ports_raw) - for key in ports: - if ports[key]['host'] == '0.0.0.0' and host_ip is not None: - ports[key]['host'] = host_ip + if host_ip is not None: + for key in ports: + if ports[key]['host'] == '0.0.0.0': + ports[key]['host'] = host_ip json.dump(ports, f) break else: From 22b1a3c7238406d63e38e6e775cec45a7e024afe Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 5 Nov 2019 20:52:43 +0100 Subject: [PATCH 5/8] Also eagerload tool_dependency to tool shed repository relation --- lib/galaxy/tools/cache.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 21fe1585654..768d1b1ab07 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -151,7 +151,9 @@ class ToolShedRepositoryCache(object): def rebuild(self): self.repositories = self.app.install_model.context.current.query(self.app.install_model.ToolShedRepository).options( defer(self.app.install_model.ToolShedRepository.metadata), - joinedload('tool_dependencies'), + joinedload('tool_dependencies').subqueryload('tool_shed_repository').options( + defer(self.app.install_model.ToolShedRepository.metadata) + ), ).all() repos_by_tuple = defaultdict(list) for repository in self.repositories + self.local_repositories: From d6b69cf6d591459949110bed3aa4ee3cb74abb77 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 6 Nov 2019 14:21:19 +0100 Subject: [PATCH 6/8] Use final_job_state in data manager exec_after_process This was all working correctly in my test, but removes some custom logic. It was theoretically possible for the old logic to fail if there was no output dataset for the job. --- lib/galaxy/jobs/__init__.py | 2 +- lib/galaxy/tools/__init__.py | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index e9816ce9be3..466a62a0c46 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -1638,7 +1638,7 @@ class JobWrapper(HasResourceParameters): # Certain tools require tasks to be completed after job execution # ( this used to be performed in the "exec_after_process" hook, but hooks are deprecated ). param_dict = self.get_param_dict(job) - self.tool.exec_after_process(self.app, inp_data, out_data, param_dict, job=job) + self.tool.exec_after_process(self.app, inp_data, out_data, param_dict, job=job, final_job_state=final_job_state) # Call 'exec_after_process' hook self.tool.call_hook('exec_after_process', self.app, inp_data=inp_data, out_data=out_data, param_dict=param_dict, diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 9fcbbbfc5e7..cce4cdddc22 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -43,6 +43,7 @@ from galaxy.tool_util.loader import ( raw_tool_xml_tree, template_macro_params ) +from galaxy.tool_util.output_checker import DETECTED_JOB_STATE from galaxy.tool_util.parser import ( get_tool_source, get_tool_source_from_representation, @@ -1747,7 +1748,7 @@ class Tool(Dictifiable): def exec_before_job(self, app, inp_data, out_data, param_dict={}): pass - def exec_after_process(self, app, inp_data, out_data, param_dict, job=None): + def exec_after_process(self, app, inp_data, out_data, param_dict, job=None, **kwds): pass def job_failed(self, job_wrapper, message, exception=False): @@ -2415,7 +2416,7 @@ class SetMetadataTool(Tool): history.id, job.user, incoming={'input1': hda}, overwrite=False ) - def exec_after_process(self, app, inp_data, out_data, param_dict, job=None): + def exec_after_process(self, app, inp_data, out_data, param_dict, job=None, **kwds): working_directory = app.object_store.get_filename( job, base_dir='job_work', dir_only=True, obj_dir=True ) @@ -2499,17 +2500,13 @@ class DataManagerTool(OutputParameterJSONTool): if self.data_manager_id is None: self.data_manager_id = self.id - def exec_after_process(self, app, inp_data, out_data, param_dict, job=None, **kwds): + def exec_after_process(self, app, inp_data, out_data, param_dict, job=None, final_job_state=None, **kwds): assert self.allow_user_access(job.user), "You must be an admin to access this tool." + if final_job_state != DETECTED_JOB_STATE.OK: + return # run original exec_after_process super(DataManagerTool, self).exec_after_process(app, inp_data, out_data, param_dict, job=job, **kwds) # process results of tool - if job and job.state == job.states.ERROR: - return - # Job state may now be 'running' instead of previous 'error', but datasets are still set to e.g. error - for dataset in out_data.values(): - if dataset.state != dataset.states.OK: - return data_manager_id = job.data_manager_association.data_manager_id data_manager = self.app.data_managers.get_manager(data_manager_id, None) assert data_manager is not None, "Invalid data manager (%s) requested. It may have been removed before the job completed." % (data_manager_id) From bb38795e75e7fbcb43794a7c23deab5c196a8e44 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 6 Nov 2019 14:32:47 +0100 Subject: [PATCH 7/8] Give test data managers distinct ids and names --- test/functional/tools/data_manager.xml | 8 +++++--- test/functional/tools/data_manager_add.xml | 10 ++++++---- test/functional/tools/data_manager_add_remove.xml | 10 ++++++---- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/test/functional/tools/data_manager.xml b/test/functional/tools/data_manager.xml index 33261485299..36891279dc4 100644 --- a/test/functional/tools/data_manager.xml +++ b/test/functional/tools/data_manager.xml @@ -2,13 +2,15 @@ {"data_tables": {"testbeta": [{"value": "newvalue", "path": "newvalue.txt"}]}} - + mkdir $out_file.files_path ; - echo "A new value" > $out_file.files_path/newvalue.txt; - cp $static_test_data $out_file + echo "A new value" > '$out_file.files_path/newvalue.txt'; + cp '$static_test_data' '$out_file'; + exit $exit_code + diff --git a/test/functional/tools/data_manager_add.xml b/test/functional/tools/data_manager_add.xml index 3ab0dae4a8d..6dcfd500d08 100644 --- a/test/functional/tools/data_manager_add.xml +++ b/test/functional/tools/data_manager_add.xml @@ -1,14 +1,16 @@ - + {"data_tables": {"testbeta": { "add": [{"value": "newvalue", "path": "newvalue.txt"}]}}} - + mkdir $out_file.files_path ; - echo "A new value" > $out_file.files_path/newvalue.txt; - cp $static_test_data $out_file + echo "A new value" > '$out_file.files_path/newvalue.txt'; + cp '$static_test_data' '$out_file'; + exit $exit_code + diff --git a/test/functional/tools/data_manager_add_remove.xml b/test/functional/tools/data_manager_add_remove.xml index 5dd69106926..1ed984f0286 100644 --- a/test/functional/tools/data_manager_add_remove.xml +++ b/test/functional/tools/data_manager_add_remove.xml @@ -1,14 +1,16 @@ - + {"data_tables": {"testbeta": { "add": [{"value": "newvalue", "path": "newvalue.txt"}, {"value": "newvalue2", "path": "newvalue2.txt"}], "remove": [{"value": "newvalue", "path": "newvalue.txt"}]}}} - + mkdir $out_file.files_path ; - echo "A new value" > $out_file.files_path/newvalue.txt; - cp $static_test_data $out_file + echo "A new value" > '$out_file.files_path/newvalue.txt'; + cp '$static_test_data' '$out_file'; + exit $exit_code + From 68eb3b000fbb31f3d24ad8db7a7ee5cbe7fe32ab Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 6 Nov 2019 16:08:05 -0500 Subject: [PATCH 8/8] if ldda message is not present but ldda info is, return it instead this is a bit of a hack but given the lack of clarity of what field does what on what object (info/message/description on ldda/ld/folder) it should help with the data presentation --- lib/galaxy/webapps/galaxy/api/folder_contents.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/api/folder_contents.py b/lib/galaxy/webapps/galaxy/api/folder_contents.py index 8b9d8050f00..799142215c8 100644 --- a/lib/galaxy/webapps/galaxy/api/folder_contents.py +++ b/lib/galaxy/webapps/galaxy/api/folder_contents.py @@ -129,6 +129,9 @@ class FolderContentsController(BaseAPIController, UsesLibraryMixin, UsesLibraryM tags=ldda_tags)) if content_item.library_dataset_dataset_association.message: return_item.update(dict(message=content_item.library_dataset_dataset_association.message)) + elif content_item.library_dataset_dataset_association.info: + # There is no message but ldda info contains something so we display that instead. + return_item.update(dict(message=content_item.library_dataset_dataset_association.info)) # For every item include the default metadata return_item.update(dict(id=encoded_id,