[Findit] Capture versionized metadata for master_flake_analysis

appengine/findit/model/flake/master_flake_analysis.py

Index: appengine/findit/model/flake/master_flake_analysis.py
diff --git a/appengine/findit/model/flake/master_flake_analysis.py b/appengine/findit/model/flake/master_flake_analysis.py
index 2805c843e5c6f43a3088f709df6e192a2ac079fc..2ada5e8a00a08babedc75af218eda9c326162d24 100644
--- a/appengine/findit/model/flake/master_flake_analysis.py
+++ b/appengine/findit/model/flake/master_flake_analysis.py
@@ -6,21 +6,17 @@ import base64
 
 from google.appengine.ext import ndb
 
-from model.base_build_model import BaseBuildModel
+from common import time_util
+

[chanli, 2016/09/27 22:15:24]

Nit: remove this empty line.

[lijeffrey, 2016/09/28 03:12:30]

Done.

+from model import analysis_status
 from model.base_analysis import BaseAnalysis
-from model.flake.flake_swarming_task import FlakeSwarmingTask
+from model.base_build_model import BaseBuildModel
+from model.versioned_model import VersionedModel
 
 
-class MasterFlakeAnalysis(BaseAnalysis, BaseBuildModel):
+class MasterFlakeAnalysis(BaseAnalysis, BaseBuildModel, VersionedModel):
   """Represents an analysis of a flaky test in a Chromium Waterfall."""
 
-  @staticmethod
-  def _CreateAnalysisId(master_name, builder_name,
-                       build_number, step_name, test_name):
-    encoded_test_name = base64.urlsafe_b64encode(test_name)
-    return '%s/%s/%s/%s/%s' % (master_name, builder_name,
-                               build_number, step_name, encoded_test_name)
-
   @ndb.ComputedProperty
   def step_name(self):
     return self.key.pairs()[0][1].split('/')[3]
@@ -30,29 +26,92 @@ class MasterFlakeAnalysis(BaseAnalysis, BaseBuildModel):
     return base64.urlsafe_b64decode(self.key.pairs()[0][1].split('/')[4])
 
   @staticmethod
-  def _CreateKey(master_name, builder_name, build_number,
-                 step_name, test_name):  # pragma: no cover
-    return ndb.Key('MasterFlakeAnalysis',
-                   MasterFlakeAnalysis._CreateAnalysisId(
-                       master_name, builder_name, build_number,
-                       step_name, test_name))
+  def _CreateAnalysisId(
+      master_name, builder_name, build_number, step_name, test_name):
+    encoded_test_name = base64.urlsafe_b64encode(test_name)
+    return '%s/%s/%s/%s/%s' % (
+        master_name, builder_name, build_number, step_name, encoded_test_name)
 
-  @staticmethod
-  def Create(master_name, builder_name, build_number,
-             step_name, test_name):  # pragma: no cover
-    return MasterFlakeAnalysis(
-        key=MasterFlakeAnalysis._CreateKey(
-            master_name, builder_name, build_number,
-            step_name, test_name))
+  # Arguments number differs from overridden method - pylint: disable=W0221
+  @classmethod
+  def Create(cls, master_name, builder_name, build_number, step_name,
+             test_name):  # pragma: no cover.
+    return super(MasterFlakeAnalysis, cls).Create(
+        MasterFlakeAnalysis._CreateAnalysisId(
+            master_name, builder_name, build_number, step_name, test_name))
 
-  @staticmethod
-  def Get(master_name, builder_name, build_number,
-          step_name, test_name):  # pragma: no cover
-    return MasterFlakeAnalysis._CreateKey(
-        master_name, builder_name, build_number, step_name, test_name).get()
+  # Arguments number differs from overridden method - pylint: disable=W0221
+  @classmethod
+  def GetVersion(cls, master_name, builder_name, build_number, step_name,
+                 test_name, version=None):  # pragma: no cover.
+    return super(MasterFlakeAnalysis, cls).GetVersion(
+        key=MasterFlakeAnalysis._CreateAnalysisId(
+            master_name, builder_name, build_number, step_name, test_name),
+        version=version)
 
-  # List of tested build_numbers and their corresponding success rates.
-  # We need to keep these sorted manually.
-  build_numbers = ndb.IntegerProperty(indexed=False, repeated=True)
-  success_rates = ndb.FloatProperty(indexed=False, repeated=True)
+  def Reset(self):
+    self.created_time = time_util.GetUTCNow()
+    self.status = analysis_status.PENDING
+    self.completed_time = None
+    self.swarming_rerun_results = {}
+    self.error = None
+    self.correct = None
+    self.algorithm_parameters = None
+    self.suspected_flake_build_number = None
+    self.build_numbers = []
+    self.pass_rates = []
+
+  # The UTC timestamp the check flake task was requested.

[stgao, 2016/09/28 00:03:24]

Let's avoid "check flake task" or similar in code. Just say "analysis"?

[lijeffrey, 2016/09/28 03:12:30]

Done.

+  created_time = ndb.DateTimeProperty(indexed=True)
+
+  # The UTC timestamp the check flake task came completed.
+  completed_time = ndb.DateTimeProperty(indexed=True)
+
+  # A dict containing information about each swarming rerun's results that can
+  # be used for metrics, such as number of cache hits, average run time, etc.

[stgao, 2016/09/28 00:03:24]

Are the metrics for running Swraming tasks or for running the whole analysis?

It seems a mixed here.

[lijeffrey, 2016/09/28 03:12:30]

It is for the whole analysis, but depends on each swarming task. Since there can
be a variety of cache hits/optimization strategies used later, this dict is to
capture each swarming task's data that was relevant to this analysis' results.
Calculating metrics will be based heavily on this dict for cache hits, number of
tasks required to come to a conclusion, average time per swarming task
normalized +/- normalized per iteration, etc.

I'll reword the description to be more clear.

+  # Example dict:
+  # {
+  #     task_id_1: {
+  #         'request_time': 2016-09-06 (10:21:26.288) UTC

[stgao, 2016/09/28 00:03:24]

Why do we need these detailed info about the Swarming tasks?

[lijeffrey, 2016/09/28 03:12:30]

This is for calculating time per iteration. Later when we implement caching,
caching can be 1 of 3-states: hit, miss, or partial. In the case of partial
(another analysis triggered a swarming task on this exact build that has started
but is not finished), without these timestamps we don't know how much the
partial cache hit is benefiting and needs to be separated from complete miss/hit
cases.

I am still thinking on how we want to handle the partial cache hit cases, which
may include introducing another timestamp for the time that this analysis
requested this swarming task, and using request_time as the time that the
swarming task was actually requested (which in cache hit cases will be before
the time that this analysis requested it), but I haven't finalized the design
yet for partial hits that will be in a follow up cl.

+  #         'start_time': 2016-09-06 (10:21:26.288) UTC,
+  #         'end_time': 2016-09-06 (10:21:26.288) UTC,
+  #         'build_number': 12345,
+  #         'cache_hit': True/False,
+  #         'number_of_iterations': 100,
+  #         'number_of_passes': 90,
+  #     },
+  #     task_id_2: {
+  #         ...
+  #     },
+  #     ...
+  # }
+  swarming_rerun_results = ndb.JsonProperty(default={}, indexed=False)

[stgao, 2016/09/28 00:03:24]

This will grow big very quickly, thus it should be compressed=True.

[lijeffrey, 2016/09/28 03:12:30]

Done.

+
+  # Error code and message, if any.
+  error = ndb.JsonProperty(indexed=False)
+
+  # Boolean whether or not the suspected regression range/build is correct.
+  correct = ndb.BooleanProperty(indexed=False)

[stgao, 2016/09/28 00:03:24]

correct_regression_range? Later we might need correct_culprit or the like.

[stgao, 2016/09/28 00:03:24]

Let's index this field for easier query.

[lijeffrey, 2016/09/28 03:12:30]

Done.

[lijeffrey, 2016/09/28 03:12:30]

Good point. Added correct_culprit and renamed this correct_regression_range.

+
+  # The look back algorithm parameters that were used, as specified in Findit's
+  # configuration. For example,
+  # {
+  #     'iterations_to_rerun': 100,
+  #     'lower_flake_threshold': 0.02,
+  #     'max_build_numbers_to_look_back': 500,
+  #     'max_flake_in_a_row': 4,
+  #     'max_stable_in_a_row': 4,
+  #     'upper_flake_threshold': 0.98
+  # }
+  algorithm_parameters = ndb.JsonProperty(indexed=False)
+
+  # The suspected build number to have introduced the flakiness.
   suspected_flake_build_number = ndb.IntegerProperty()
+
+  # The build numbers that were examined to generate this run's flakiness graph.
+  # This list needs to be kept sorted manually.
+  build_numbers = ndb.IntegerProperty(indexed=False, repeated=True)
+
+  # The corresponding pass rates of build number's swarming rerun results.
+  # This list needs to be kept sorted manually.
+  pass_rates = ndb.FloatProperty(indexed=False, repeated=True)