From 121285b40be5d7233af33b40045a9c2ffb5a9c46 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Fri, 15 Dec 2017 08:34:11 -0500
Subject: [PATCH] Let ToolProvidedMetadata interface more directly decide if it
 has failed outputs.

I like this better for three reasons:

- Since usually it is scripts producing this JSON - we have the most control at that point for determining the failure and we don't have to deal with an artificial dependency between the tool's stdio and the output.
- At some point we could potentially allow some datasets to be ok now even though the job fails.
- It is a cleaner interface at the Python level between job finish and output collection IMO (no need for isinstance checking).
---
 lib/galaxy/jobs/__init__.py                   | 17 +----------------
 lib/galaxy/tools/parameters/output_collect.py | 19 +++++++++++++++++++
 tools/data_source/upload.py                   |  3 ++-
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py
index 745aeb768482..420f29d4d521 100644
--- a/lib/galaxy/jobs/__init__.py
+++ b/lib/galaxy/jobs/__init__.py
@@ -27,10 +27,6 @@
 from galaxy.jobs.actions.post import ActionBox
 from galaxy.jobs.mapper import JobRunnerMapper
 from galaxy.jobs.runners import BaseJobRunner, JobState
-from galaxy.tools.parameters.output_collect import (
-    LegacyToolProvidedMetadata,
-    ToolProvidedMetadata
-)
 from galaxy.util import safe_makedirs, unicodify
 from galaxy.util.bunch import Bunch
 from galaxy.util.expressions import ExpressionContext
@@ -1184,17 +1180,6 @@ def finish(
 
         # We collect the stderr from tools that write their stderr to galaxy.json
         tool_provided_metadata = self.get_tool_provided_job_metadata()
-        extra_stderr = ''
-        if isinstance(tool_provided_metadata, LegacyToolProvidedMetadata):
-            extra_stderr = [item.get('stderr') for item in tool_provided_metadata.tool_provided_job_metadata if item.get('stderr')]
-        elif isinstance(tool_provided_metadata, ToolProvidedMetadata):
-            extra_stderr = [item.get('stderr') for item in tool_provided_metadata.tool_provided_job_metadata.values() if item.get('stderr')]
-        if extra_stderr:
-            extra_stderr = "\n".join(extra_stderr)
-            if stderr:
-                stderr = "%s\n%s" % (stderr, extra_stderr)
-            else:
-                stderr = extra_stderr
 
         # Check the tool's stdout, stderr, and exit code for errors, but only
         # if the job has not already been marked as having an error.
@@ -1202,7 +1187,7 @@ def finish(
 
         # We set final_job_state to use for dataset management, but *don't* set
         # job.state until after dataset collection to prevent history issues
-        if self.check_tool_output(stdout, stderr, tool_exit_code, job):
+        if self.check_tool_output(stdout, stderr, tool_exit_code, job) and not tool_provided_metadata.has_failed_outputs():
             final_job_state = job.states.OK
         else:
             final_job_state = job.states.ERROR
diff --git a/lib/galaxy/tools/parameters/output_collect.py b/lib/galaxy/tools/parameters/output_collect.py
index cefa90132e9c..98c628f67f0e 100644
--- a/lib/galaxy/tools/parameters/output_collect.py
+++ b/lib/galaxy/tools/parameters/output_collect.py
@@ -32,6 +32,9 @@ def get_new_datasets(self, output_name):
     def get_new_dataset_meta_by_basename(self, output_name, basename):
         return {}
 
+    def has_failed_outputs(self):
+        return False
+
 
 class LegacyToolProvidedMetadata(object):
 
@@ -74,6 +77,14 @@ def get_new_datasets(self, output_name):
         log.warning("Called get_new_datasets with legacy tool metadata provider - that is unimplemented.")
         return []
 
+    def has_failed_outputs(self):
+        found_failed = False
+        for meta in self.tool_provided_job_metadata:
+            if meta.get("failed", False):
+                found_failed = True
+
+        return found_failed
+
 
 class ToolProvidedMetadata(object):
 
@@ -112,6 +123,14 @@ def _elements_to_datasets(self, elements, level=0):
                 extra_kwds.update(element)
                 yield extra_kwds
 
+    def has_failed_outputs(self):
+        found_failed = False
+        for meta in self.tool_provided_job_metadata.values():
+            if meta.get("failed", False):
+                found_failed = True
+
+        return found_failed
+
 
 def collect_dynamic_collections(
     tool,
diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py
index 3011e5f9967a..92dd6afd7c1f 100644
--- a/tools/data_source/upload.py
+++ b/tools/data_source/upload.py
@@ -43,7 +43,8 @@ def file_err(msg, dataset, json_file):
     json_file.write(dumps(dict(type='dataset',
                                ext='data',
                                dataset_id=dataset.dataset_id,
-                               stderr=msg)) + "\n")
+                               stderr=msg,
+                               failed=True)) + "\n")
     # never remove a server-side upload
     if dataset.type in ('server_dir', 'path_paste'):
         return