[Findit] Pipeline change to save suspected cls to data store.

appengine/findit/waterfall/suspected_cl_util.py

+# Copyright 2016 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 google.appengine.ext import ndb
+
+from common.waterfall import failure_type
+from model import analysis_approach_type
+from model.wf_suspected_cl import WfSuspectedCL
+
+def _GetsStatusFromSameFailure(builds, failures):
+  for build in builds.values():
+    if build['status'] is not None and build['failures'] == failures:
+      return build['status']
+  return None
+
+
+@ndb.transactional
+def UpdateSuspectedCL(
+  repo_name, revision, commit_position,
+  approach, master_name, builder_name, build_number, cl_failure_type,
+  failures, top_score):
+
+  suspected_cl = (
+      WfSuspectedCL.Get(repo_name, revision) or
+      WfSuspectedCL.Create(repo_name, revision, commit_position))
+
+  if approach not in suspected_cl.approaches:
+    suspected_cl.approaches.append(approach)
+  if cl_failure_type not in suspected_cl.failure_type:
+    suspected_cl.failure_type.append(cl_failure_type)
+
+  build_key = '%s/%s/%d' %(master_name, builder_name, build_number)

[stgao, 2016/09/23 18:56:54]

Can we extract this logic to some place to share with other code?

The encoding and decoding should be abstracted in one place, instead of
spreading in multiple place in the code.

[chanli, 2016/09/24 00:05:00]

Done.

+  if build_key not in suspected_cl.builds:
+    build = {
+        'approaches': [approach],
+        'failure_type': cl_failure_type,
+        'failures': failures,
+        'status': _GetsStatusFromSameFailure(suspected_cl.builds, failures),
+        'top_score': top_score
+    }
+    suspected_cl.builds[build_key] = build

[stgao, 2016/09/23 18:30:21]

Inline it in #35?

[chanli, 2016/09/24 00:05:00]

Done.

+  else:
+    build = suspected_cl.builds[build_key]
+    build['approaches'].append(approach)

[stgao, 2016/09/23 18:30:21]

What if the approach is already in the list?

[chanli, 2016/09/24 00:05:00]

This is for a specific build, so logically the approach should not be in the
list (In case of re-run, we should remove all data for that build before
re-run). I can add the check but to me it's not necessary.

+
+  suspected_cl.put()