[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): write complete coverage tests for this.
 def FinditForClientID(client_id): # pragma: no cover
   """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 72 matching lines @@
   # ``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()
 
   # TODO(http://crbug.com/659346): we misplaced the coverage test; find it!
   # Arguments number differs from overridden method - pylint: disable=W0221
-  def run(self):
+  def run(self, *_args, **_kargs):

[wrengr, 2016/11/03 22:17:58]

Hrm, this should've been fixed in the landed version of 2455053004 (along with
adding a docstring explaining why the arguments are discarded). Can you rebase?

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

Done.

     # TODO(wrengr): shouldn't this method somehow call _NeedsNewAnalysis
     # to guard against race conditions?
     analysis = self._findit.GetAnalysis(self._crash_identifiers)
 
     # Update the model's status to say we're in the process of doing analysis.
     analysis.pipeline_status_path = self.pipeline_status_path()
     analysis.status = analysis_status.RUNNING
     analysis.started_time = time_util.GetUTCNow()
     analysis.findit_version = appengine_util.GetCurrentVersion()
     analysis.put()
@@ skipping 24 matching lines @@
 
 
 class PublishResultPipeline(CrashBasePipeline):
   # TODO(http://crbug.com/659346): we misplaced the coverage test; find it!
   def finalized(self):
     if self.was_aborted:  # pragma: no cover.
       logging.error('Failed to publish %s analysis result for %s',
                     repr(self._crash_identifiers), self.client_id)
 
   # Arguments number differs from overridden method - pylint: disable=W0221
-  def run(self):
+  def run(self, *_args, **_kargs):

[wrengr, 2016/11/03 22:17:58]

ditto

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

Done.

     analysis = self._findit.GetAnalysis(self._crash_identifiers)
     result = analysis.ToPublishableResult(self._crash_identifiers)
     messages_data = [json.dumps(result, sort_keys=True)]
 
     # TODO(http://crbug.com/659354): remove Findit's dependency on CrashConfig.
     client_config = self._findit.config
     # TODO(katesonia): Clean string uses in config.
     topic = client_config['analysis_result_pubsub_topic']
     pubsub_util.PublishMessagesToTopic(messages_data, topic)
     logging.info('Published %s analysis result for %s', self.client_id,
@@ skipping 12 matching lines @@
   because the ``run`` and ``finalized`` methods are executed in different
   processes.
   """
   def __init__(self, client_id, crash_identifiers):
     super(CrashWrapperPipeline, self).__init__(client_id, crash_identifiers)
     self._crash_identifiers = crash_identifiers
     self._client_id = client_id
 
   # TODO(http://crbug.com/659346): write coverage tests.
   # Arguments number differs from overridden method - pylint: disable=W0221
-  def run(self): # pragma: no cover
+  def run(self, *_args, **_kargs): # pragma: no cover

[wrengr, 2016/11/03 22:17:58]

ditto

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

Done.

     run_analysis = yield CrashAnalysisPipeline(
         self._client_id, self._crash_identifiers)
     with pipeline.After(run_analysis):
       yield PublishResultPipeline(self._client_id, self._crash_identifiers)