[Findit] Add three sub-pipelines to analyze build failure.

appengine/findit/waterfall/extract_signal_pipeline.py

Index: appengine/findit/waterfall/extract_signal_pipeline.py
diff --git a/appengine/findit/waterfall/extract_signal_pipeline.py b/appengine/findit/waterfall/extract_signal_pipeline.py
new file mode 100644
index 0000000000000000000000000000000000000000..9df0f8641e150cb0134f8ac8b46a7d0a6aecce45
--- /dev/null
+++ b/appengine/findit/waterfall/extract_signal_pipeline.py
@@ -0,0 +1,74 @@
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import logging
+
+from pipeline_utils.appengine_third_party_pipeline_src_pipeline import pipeline
+
+from common.http_client_appengine import HttpClientAppengine as HttpClient
+from model.step import Step
+from waterfall import buildbot
+from waterfall import extractors
+from waterfall import lock_util
+from waterfall.base_pipeline import BasePipeline
+
+
+class ExtractSignalPipeline(BasePipeline):
+  """A pipeline to extract failure signals from each failed step.
+
+  Input:
+    It is output of pipeline DetectFirstFailurePipeline.
+
+  Output:
+    It is a json like below:

[qyearsley, 2015/01/15 21:15:03]

The below is a dict, not JSON. It is true that the return value of a pipeline's
run() method is converted to JSON at some point, but what is returned directly
from the run() method is a dict.

[stgao, 2015/01/16 20:21:39]

Good point.
Docstring was updated.

+    {
+      'step_name1': waterfall.failure_signal.FailureSignal.ToJson(),
+      ...
+    }
+  """
+
+  HTTP_CLIENT = HttpClient()
+
+  # Arguments number differs from overridden method - pylint: disable=W0221
+  def run(self, failure_info):

[qyearsley, 2015/01/15 21:15:03]

Rather than putting Input and Output above, I suggest adding a docstring for the
run() method, which should have Args and Returns sections.

[stgao, 2015/01/16 20:21:39]

Done.

+    signals = {}
+
+    master_name = failure_info['master_name']
+    builder_name = failure_info['builder_name']
+    build_number = failure_info['build_number']
+    for step_name in failure_info['failed_steps']:
+      step = Step.GetStep(master_name, builder_name, build_number, step_name)
+      if step and step.log_data:
+        stdio_log = step.log_data
+      else:
+        if not lock_util.WaitUntilDownloadAllowed(
+            master_name):  # pragma: no cover
+          raise pipeline.Retry('Failed to pull stdio of step %s of master %s'
+                               % (step_name, master_name))
+
+        # TODO: do test-level analysis instead of step-level.
+        stdio_log = buildbot.GetStepStdio(
+            master_name, builder_name, build_number, step_name,
+            self.HTTP_CLIENT)
+        if not stdio_log:  # pragma: no cover
+          raise pipeline.Retry('Failed to pull stdio of step %s of master %s'
+                               % (step_name, master_name))
+
+        # Save stdio in datastore and avoid downloading again during retry.
+        if not step:  # pragma: no cover
+          step = Step.CreateStep(
+              master_name, builder_name, build_number, step_name)
+
+        step.log_data = stdio_log
+        try:
+          step.put()
+        except Exception as e:  # pragma: no cover
+          # Sometimes, the stdio log is too large to save in datastore.
+          logging.exception(e)
+
+      # TODO: save result in datastore?
+      signals[step_name] = extractors.ExtractSignal(
+          master_name, builder_name, step_name, None, stdio_log).ToJson()

[qyearsley, 2015/01/15 21:15:03]

Looking at the FailureSignal.ToJson method[1] - it doesn't return JSON, it
returns a dict (which is JSON-serializable, but it is not a string that contains
JSON-formatted data). This is a bit misleading.

I think that method should be renamed to ToDict.

[1]
https://code.google.com/p/chromium/codesearch#chromium/infra/appengine/findit...

[stgao, 2015/01/16 20:21:39]

Good point.
But let's do the rename in a separate CL to avoid making this CL even larger.

[qyearsley, 2015/01/16 22:55:25]

SGTM

+
+    return signals