[Findit] Change the criteria of sending notification to code review.

appengine/findit/waterfall/identify_try_job_culprit_pipeline.py

Index: appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
diff --git a/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py b/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
index 12be524fe7fe94d697e053b80b743b824dd5c3b3..1f7bb8f135f2bc014b989d64f0c609fd405f6438 100644
--- a/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
+++ b/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
@@ -182,16 +182,27 @@ def _GetCulpritsForTestsFromResultsDict(blame_list, test_results):
   return culprit_map, list(failed_revisions)
 
 
-def _NotifyCulprits(master_name, builder_name, build_number, culprits):
+def _GetHeuristicSuspectedCLs(analysis):
+  """Gets revisions of suspected cls found by heuristic approach."""
+  if analysis and analysis.suspected_cls:
+    return [(cl['repo_name'], cl['revision']) for cl in analysis.suspected_cls]
+  return []
+
+
+def _NotifyCulprits(master_name, builder_name, build_number, culprits,
+                    heuristic_cls):
   """Sends notifications to the identified culprits."""
-  try:
-    for culprit in (culprits or {}).itervalues():
-      pipeline = SendNotificationForCulpritPipeline(
+
+  for culprit in (culprits or {}).itervalues():
+    if ((culprit['repo_name'], culprit['revision']) in heuristic_cls):

[lijeffrey, 2016/09/02 06:37:34]

nit: it looks like there are some unnecessary parenthesis around this if

[chanli, 2016/09/02 16:57:19]

Done.

+      try:
+        pipeline = SendNotificationForCulpritPipeline(
           master_name, builder_name, build_number,
           culprit['repo_name'], culprit['revision'])
-      pipeline.start()
-  except Exception:  # pragma: no cover.
-    logging.exception('Failed to notify culprits.')
+        pipeline.start()
+      except Exception:  # pragma: no cover.
+        logging.exception(
+            'Failed to notify culprits which was found by both approaches.')
 
 
 class IdentifyTryJobCulpritPipeline(BasePipeline):
@@ -324,7 +335,6 @@ class IdentifyTryJobCulpritPipeline(BasePipeline):
 
       # Update analysis result and suspected CLs with results of this try job if
       # culprits were found.
-      analysis = WfAnalysis.Get(master_name, builder_name, build_number)
       updated_result_status = _GetResultAnalysisStatus(analysis, result)
       updated_suspected_cls = _GetSuspectedCLs(analysis, result)
 
@@ -334,10 +344,16 @@ class IdentifyTryJobCulpritPipeline(BasePipeline):
         analysis.suspected_cls = updated_suspected_cls
         analysis.put()
 
-    # Store try-job results.
+      # Store try-job results.
     UpdateTryJobResult()
+
+
+    analysis = WfAnalysis.Get(master_name, builder_name, build_number)
+    heuristic_cls = _GetHeuristicSuspectedCLs(analysis)
+
     # Add try-job results to WfAnalysis.
     UpdateWfAnalysisWithTryJobResult()
 
-    _NotifyCulprits(master_name, builder_name, build_number, culprits)
+    _NotifyCulprits(master_name, builder_name, build_number, culprits,
+                    heuristic_cls)
     return result.get('culprit') if result else None