[Predator] Refactor ToPublishResult and fix keyerror 'found'

appengine/findit/model/crash/crash_analysis.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 hashlib
 import json
 import logging
 
 from google.appengine.ext import ndb
 
-from common import appengine_util
 from crash.type_enums import CrashClient
 from model import analysis_status
 from model import triage_status
 
-# TODO(katesonia): Move this to fracas config.
-_FINDIT_FRACAS_FEEDBACK_URL_TEMPLATE = '%s/crash/fracas-result-feedback?key=%s'
 
 class CrashAnalysis(ndb.Model):
   """Base class to represent an analysis of a Chrome/Clusterfuzz crash."""
   ################### Properties for the crash itself. ###################
   # In which version or revision of Chrome the crash occurred. Either a version
   # number for Chrome build or a git commit hash/position for chromium build.
   crashed_version = ndb.StringProperty(indexed=False)
 
   # The stack_trace_string.
   stack_trace = ndb.StringProperty(indexed=False)
@@ skipping 120 matching lines @@
         json.dumps(crash_identifiers, sort_keys=True)).hexdigest())
 
   @classmethod
   def Get(cls, crash_identifiers):
     return cls._CreateKey(crash_identifiers).get()
 
   @classmethod
   def Create(cls, crash_identifiers):
     return cls(key=cls._CreateKey(crash_identifiers))
 
+  def ProcessResultForPublishing(self, result):
+    """Client specific processing of result data for publishing."""
+    raise NotImplementedError()
+
   def ToPublishableResult(self, crash_identifiers):
     """Convert this datastore analysis into a publishable result.
 
+    Note, this function must be called by concret subclass of CrashAnalysis

[wrengr, 2016/11/29 22:11:12]

"concret" -> "a concrete"

[Sharu Jiang, 2016/11/30 02:37:00]

Done.

[stgao, 2016/11/30 04:35:07]

I don't think this is actually done. The misspell still exists.

[Sharu Jiang, 2016/11/30 18:14:07]

Oops, done.

+    which implements ProcessResultForPublishing abstract function.

[wrengr, 2016/11/29 22:11:12]

-> "which implements the ProcessResultForPublishing method."

[Sharu Jiang, 2016/11/30 02:37:00]

Done.

+
     Args:
-      crash_identifiers (dict): ??
+      crash_identifiers (dict): Dict containing identifiers that can uniquely
+        identify CrashAnalysis entity.
 
     Returns:
       A dict of the given ``crash_identifiers``, this model's
       ``client_id``, and a publishable version of this model's ``result``.
     """
     result = copy.deepcopy(self.result)
-    client_id = self.client_id
+    if result.get('found') and 'suspected_cls' in result:
+      for cl in result['suspected_cls']:
+        cl['confidence'] = round(cl['confidence'], 2)
+        cl.pop('reason', None)
 
-    # TODO(katesonia): move this to ChromeCrashAnalysis
-    if (client_id == CrashClient.FRACAS or
-        client_id == CrashClient.CRACAS):
-      result['feedback_url'] = _FINDIT_FRACAS_FEEDBACK_URL_TEMPLATE % (
-          appengine_util.GetDefaultVersionHostname(), self.key.urlsafe())
-      if result['found'] and 'suspected_cls' in result:
-        for cl in result['suspected_cls']:
-          cl['confidence'] = round(cl['confidence'], 2)
-          cl.pop('reason', None)
-    elif client_id == CrashClient.CLUSTERFUZZ:  # pragma: no cover.
-      # TODO(katesonia): Post process clusterfuzz model result if needed.
-      pass
-
+    result = self.ProcessResultForPublishing(result)
     logging.info('Publish result:\n%s',
                  json.dumps(result, indent=4, sort_keys=True))
     return {
         'crash_identifiers': crash_identifiers,
-        'client_id': client_id,
+        'client_id': self.client_id,

[wrengr, 2016/11/29 22:11:12]

Since you removed the line where we initialized self.client_id (170 in master),
it may not be defined here

[Sharu Jiang, 2016/11/30 02:37:00]

I think the self.client_id would always exists since it's a property of
CrashAnalysis.

         'result': result,
     }