[Predator] Move time_util from common/ to lib/, split code review related part to code_review_util

appengine/findit/crash/crash_pipeline.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.
 
 import copy
 import json
 import logging
 
 from common import appengine_util
 from common import pubsub_util
-from common import time_util
 from common.http_client_appengine import HttpClientAppengine
 from common.pipeline_wrapper import BasePipeline
 from common.pipeline_wrapper import pipeline
 from crash import findit_for_chromecrash
 from crash import findit_for_clusterfuzz
 from crash.type_enums import CrashClient
+from lib import time_util
 from lib.gitiles import gitiles_repository
 from model import analysis_status
 
 
 # TODO(http://crbug.com/659346): this needs complete coverage tests.
 def FinditForClientID(client_id):
   """Construct a Findit object from a client id string specifying the class.
 
   We cannot pass Findit objects to the various methods in
   ``crash.crash_pipeline``, because they are not JSON serializable. For
@@ skipping 80 matching lines @@
   # N.B., this method must be factored out for unittest reasons; since
   # ``finalized`` takes no arguments (by AppEngine's spec) and
   # ``was_aborted`` can't be altered directly.
   def _PutAbortedError(self):
     """Update the ndb.Model to indicate that this pipeline was aborted."""
     logging.error('Aborted analysis for %s', repr(self._crash_identifiers))
     analysis = self._findit.GetAnalysis(self._crash_identifiers)
     analysis.status = analysis_status.ERROR
     analysis.put()
 
-  def run(self, *_args, **_kwargs):
+  # TODO(http://crbug.com/659346): we misplaced the coverage test; find it!
+  # Arguments number differs from overridden method - pylint: disable=W0221
+  def run(self, *_args, **_kargs):

[stgao, 2016/11/03 22:14:05]

nit: _kwargs

[Sharu Jiang, 2016/11/04 01:02:35]

Done.

     """Call predator to do the analysis of the given crash.
 
     N.B., due to the structure of AppEngine pipelines, this method must
     accept the same arguments as are passed to ``__init__``; however,
     because they were already passed to ``__init__`` there's no use in
     recieving them here. Thus, we discard all the arguments to this method
     (except for ``self``, naturally).
     """
     # TODO(wrengr): shouldn't this method somehow call _NeedsNewAnalysis
     # to guard against race conditions?
@@ skipping 85 matching lines @@
     N.B., due to the structure of AppEngine pipelines, this method must
     accept the same arguments as are passed to ``__init__``; however,
     because they were already passed to ``__init__`` there's no use in
     recieving them here. Thus, we discard all the arguments to this method
     (except for ``self``, naturally).
     """
     run_analysis = yield CrashAnalysisPipeline(
         self._client_id, self._crash_identifiers)
     with pipeline.After(run_analysis):
       yield PublishResultPipeline(self._client_id, self._crash_identifiers)