Waterfall components of regression range finder.

appengine/findit/waterfall/flake/initialize_flake_pipeline.py

+# Copyright 2014 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 datetime import datetime
+import logging
+
+from google.appengine.ext import ndb
+
+from common import appengine_util
+from common import constants
+from model.flake.flake_swarming_task import FlakeSwarmingTask
+from model.flake.master_flake_analysis import MasterFlakeAnalysis
+from model.flake.flake_swarming_task import FlakeSwarmingTask
+from waterfall.flake.recursive_flake_pipeline import RecursiveFlakePipeline
+from model import analysis_status

[chanli, 2016/07/08 16:58:54]

Move ln 16 before ln 10

[stgao, 2016/07/09 00:04:33]

imports of the same category are usually ordered alphabetically

[caiw, 2016/07/14 00:59:44]

Done.

[caiw, 2016/07/14 00:59:44]

Done.

+
+
+

[chanli, 2016/07/08 16:58:54]

Only 2 empty lines are needed here.

[caiw, 2016/07/14 00:59:44]

Done.

+@ndb.transactional
+# Unused argument - pylint: disable=W0613
+def NeedANewAnalysis(
+    master_name, builder_name, step_name, build_number):

[chanli, 2016/07/08 16:58:53]

builder_name is not used in function?

[caiw, 2016/07/14 00:59:43]

Done.

+  """Checks status of analysis for the test and decides if a new one is needed.
+
+  A MasterFlakeAnalysis entity for the given build will be created if none

[chanli, 2016/07/08 16:58:54]

I thought MasterFlakeAnalysis is by (master_name, builder_name, step_name)?
Although as I mentioned in CL 1, I think we need this at test level rather than
step level.

[caiw, 2016/07/14 00:59:44]

Done.

+  exists. When a new analysis is needed, this function will create and
+  save a MasterFlakeAnalysis entity to the datastore, or it will
+  reset the existing one but still keep the result of last analysis.
+
+  Returns:
+    True if an analysis is needed, otherwise False.
+  """
+  # Where should we put this logic?
+  # I think this flow has some redundancy with check_flake handler

[stgao, 2016/07/09 00:04:33]

The logic to determine whether an analysis is needed or not should be here
within a NDB transaction.

The check_flake handler could just call function ``ScheduleAnalysisIfNeeded``
below regardless to which status the analysis is, and then render the result
based on the MasterFlakeAnalysis returned from the function.

[caiw, 2016/07/14 00:59:44]

Done.

+
+  analysis = MasterFlakeAnalysis.Get(master_name, builder_name, step_name)
+  # If no analysis exists, create it and return True.
+  # For debugging purposes, we want to create a new one
+  # as long as it is not completed

[chanli, 2016/07/08 16:58:54]

It is fine for now that you have this kind of comments in code since the code is
still under development, but make sure remove them when you're ready to commit
you code.

[caiw, 2016/07/14 00:59:43]

Done.

+
+  if not analysis:
+    analysis = MasterFlakeAnalysis.Create(
+        master_name, builder_name, step_name)
+    analysis.status = analysis_status.PENDING
+    analysis.put()
+    return True
+  elif (analysis.status != analysis_status.COMPLETED):

[chanli, 2016/07/08 16:58:54]

what if analysis.status == PENDING or RUNNING?

[caiw, 2016/07/14 00:59:43]

Done.

+    # If the analysis failed, destroy it and create a new one.
+    # if analysis.status == analysis_status.ERROR:
+    MasterFlakeAnalysis.Get(master_name, builder_name, step_name).key.delete()

[chanli, 2016/07/08 16:58:54]

Alternative: write a Reset function instead.

[caiw, 2016/07/14 00:59:44]

Done.

+    analysis = MasterFlakeAnalysis.Create(
+        master_name, builder_name, step_name)
+    analysis.status = analysis_status.PENDING
+    analysis.put()
+    return True
+
+  # If the analysis finished, return False.
+  if analysis.status == analysis_status.COMPLETED:

[chanli, 2016/07/08 16:58:54]

This is actually else after elif above, right?

[caiw, 2016/07/14 00:59:44]

Done.

+    return False
+
+# Unused arguments - pylint: disable=W0612, W0613
+def ScheduleAnalysisIfNeeded(master_name, builder_name, step_name, build_number,
+                             testcase, force=False,
+                             queue_name=constants.DEFAULT_QUEUE):
+  """Schedules an analysis if needed and returns the MasterFlakeAnalysis.
+
+  When the build failure was already analyzed and a new analysis is scheduled,
+  the returned WfAnalysis will still have the result of last completed analysis.
+
+  Args:
+    master_name (str): The master name of the failed test
+    builder_name (str): The builder name of the failed test
+    build_number (int): The build number of the failed test
+    step_name (str): The name of the test suite
+
+  Returns:
+    A MasterFlakeAnalysis instance.
+  """
+  # Change force to true for testing?
+  if NeedANewAnalysis(
+      master_name, builder_name, step_name, build_number):
+    pipeline_job = RecursiveFlakePipeline(
+        master_name, builder_name, step_name, build_number, testcase)
+    pipeline_job.target = appengine_util.GetTargetNameForModule(
+        constants.WATERFALL_BACKEND)
+    pipeline_job.start(queue_name=queue_name)
+  return MasterFlakeAnalysis.Get(
+      master_name, builder_name, step_name)