[Findit] Group failures by culprit and send notification to codereview.

appengine/findit/waterfall/identify_try_job_culprit_pipeline.py

 # 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.
 
 from collections import defaultdict
+import logging
 
 from google.appengine.ext import ndb
 
 from common.git_repository import GitRepository
 from common.http_client_appengine import HttpClientAppengine as HttpClient
 from common.pipeline_wrapper import BasePipeline
 from model import analysis_status
 from model import result_status
 from model.wf_analysis import WfAnalysis
 from model.wf_try_job import WfTryJob
 from model.wf_try_job_data import WfTryJobData
 from waterfall.try_job_type import TryJobType
+from waterfall.send_notification_for_culprit_pipeline import \

[lijeffrey, 2016/06/21 00:07:34]

nit: I think gpylint doesn't like multi-line lines using \

is it possible to do something like:

from waterfall.send_notification_for_culprit_pipeline import (
    SendNotificationForCulpritPipeline)?

[stgao, 2016/06/21 15:14:29]

Done.

+    SendNotificationForCulpritPipeline
 
 
 GIT_REPO = GitRepository(
     'https://chromium.googlesource.com/chromium/src.git', HttpClient())
 
 
 def _GetResultAnalysisStatus(analysis, result):
   """Returns the analysis status based on existing status and try job result.
 
   Args:
@@ skipping 62 matching lines @@
     else:
       for test_cl_info in results['tests'].values():
         revision = test_cl_info.get('revision')
         if revision not in suspected_cl_revisions:
           suspected_cl_revisions.append(revision)
           suspected_cls.append(test_cl_info)
 
   return suspected_cls
 
 
+def _NotifyCulprits(master_name, builder_name, build_number, culprits):
+  """Sends notifications to the identified culprits."""
+  try:
+    for culprit in (culprits or {}).itervalues():
+      pipeline = SendNotificationForCulpritPipeline(
+          master_name, builder_name, build_number,
+          culprit['repo_name'], culprit['revision'])
+      pipeline.start()
+  except Exception:
+    logging.exception('Failed to notify culprits.')
+
+
 class IdentifyTryJobCulpritPipeline(BasePipeline):
   """A pipeline to identify culprit CL info based on try job compile results."""
 
   def _GetCulpritInfo(self, failed_revisions):
     """Gets commit_positions and review urls for revisions."""
     culprits = {}
     # TODO(lijeffrey): remove hard-coded 'chromium' when DEPS file parsing is
     # supported.
     for failed_revision in failed_revisions:
       culprits[failed_revision] = {
@@ skipping 209 matching lines @@
           analysis.suspected_cls != updated_suspected_cls):
         analysis.result_status = updated_result_status
         analysis.suspected_cls = updated_suspected_cls
         analysis.put()
 
     # Store try-job results.
     UpdateTryJobResult()
     # Add try-job results to WfAnalysis.
     UpdateWfAnalysisWithTryJobResult()
 
+    _NotifyCulprits(master_name, builder_name, build_number, culprits)

[chanli, 2016/06/21 17:50:31]

So only culprits found by try jobs will be included, right? And that's expected?

If so, when do you want to enable this change in prod? I mean, should we wait
for the accuracy rate for try jobs to get higher?

[stgao, 2016/06/24 16:10:28]

That's what I thought. Are you proposing heuristic results should be notified
too?
As in the other pipeline, we only notify if the same culprit is identified for
2+ different builders. I think that's good enough to avoid the false positive
caused by flaky compile.
Did you find some cases that it won't work?

I was supposed to add a flag to disable the notification.
But it will be in a separate CL.

[chanli, 2016/06/24 23:42:38]

Such as the case where the culprit revision is skipped by analyze? 

     return result.get('culprit') if result else None