[Findit] Add iterator and crash_iterator for delta test

appengine/findit/util_scripts/crash_queries/client_crash_iterator.py

Index: appengine/findit/util_scripts/crash_queries/client_crash_iterator.py
diff --git a/appengine/findit/util_scripts/crash_queries/client_crash_iterator.py b/appengine/findit/util_scripts/crash_queries/client_crash_iterator.py
new file mode 100644
index 0000000000000000000000000000000000000000..0945c4c2a32802bcac396e305962bee919a83fd8
--- /dev/null
+++ b/appengine/findit/util_scripts/crash_queries/client_crash_iterator.py
@@ -0,0 +1,111 @@
+# 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 base64
+import copy
+from datetime import date
+from datetime import datetime
+from datetime import timedelta
+import logging
+import json
+import os
+import zlib
+
+import dev_appserver
+dev_appserver.fix_sys_path()
+
+from crash import findit_for_client
+from crash.type_enums import CrashClient
+from crash_queries import crash_iterator
+from model.crash.fracas_crash_analysis import FracasCrashAnalysis
+from model.crash.cracas_crash_analysis import CracasCrashAnalysis
+import remote_api
+
+_DEFAULT_BATCH_SIZE = 1000
+_TODAY = date.today().strftime('%Y-%m-%d')
+_A_YEAR_AGO = (date.today() - timedelta(days=365)).strftime('%Y-%m-%d')
+
+COMMON_CRASH_INFO_FIELDS = ['crashed_version', 'stack_trace', 'signature',
+                            'platform', 'client_id']
+
+
+def GetCrashInfoFields(client_id):

[wrengr, 2016/10/05 18:41:58]

The objects encapsulating the various CrashClients should keep track of their
own info-field sets. Putting this stuff here introduces too much coupling and
will make it harder to refactor later.

[Sharu Jiang, 2016/10/06 18:16:56]

There is no need for this function any more, since the channal and
historical_metadata can be included in customized_data, just used the
COMMON_CRASH_INFO_FIELDS for all the clients.

We can still have a function to get the COMMON_CRASH_INFO_FIELDS in the objects
encapsulating the various CrashClients, however we don't want that class knowing
what info-fields delta test needs to run. 

+  """Gets needed fields for crash info for client.
+
+  The crash info fields are the selected fields from a crash analysis object.
+  Soa crash analysis can be converted to dict.
+  """
+  crash_info_fields = copy.deepcopy(COMMON_CRASH_INFO_FIELDS)
+  if client_id == CrashClient.FRACAS or client_id == CrashClient.CRACAS:
+    crash_info_fields.append('channel')
+    crash_info_fields.append('historical_metadata')
+  elif client_id == CrashClient.CLUSTERFUZZ:
+    # TODO(katesonia): Define CluterfuzzCrashAnalysis.
+    pass
+
+  return crash_info_fields
+
+
+def GetAnalysisClassForClient(client_id):

[wrengr, 2016/10/05 18:41:58]

Again, this is too fragile and introduces unnecessary dependencies. There should
be a CrashClient class, instances of which know their own id and have a method
for returning the appropriate class. Putting all this sort of stuff into such a
class (a) makes it obvious who has responsability for doing what, and (b)
ensures that client code —like this file— need not worry about the details of
how that works.

[Sharu Jiang, 2016/10/06 18:16:56]

Added a TODO.

[wrengr, 2016/10/06 19:20:08]

FWIW, I'm in the progress of combining findit_for_*.py and making the general
class that captures all these sorts of things over there. I'll try to publish
that CL later today. After one or the other of our CLs land, the other can be
modified to update the code here to work with that new organization.

In the long run, the general class I'm defining in that CL should move to
./findit/appengine rather than staying in ./findit/azalea, since it depends on
ndb specifics. (I'm not sure it'd be profitable to have a variant of the class
with the ndb stuff abstracted out.) Before doing that some details about the
azalea API still need to be worked out though, since azalea will want to
produce/consume objects/tuples which capture much of the same information cached
in various ndb.Models.

+  if client_id == CrashClient.FRACAS:
+    return FracasCrashAnalysis
+  elif client_id == CrashClient.CRACAS:
+    return CracasCrashAnalysis
+  elif client_id == CrashClient.CLUSTERFUZZ:
+    # TODO(katesonia): Define ClusterfuzzCrashAnalysis.
+    return None
+
+  return None
+
+
+def GetQueryForClient(client_id, property_values, start_date, end_date,
+                      datetime_pattern='%Y-%m-%d'):
+  if property_values is None:
+    property_values = {}
+
+  start_date = datetime.strptime(start_date, datetime_pattern)
+  end_date = datetime.strptime(end_date, datetime_pattern)
+  cls = GetAnalysisClassForClient(client_id)
+  query = cls.query(**property_values)
+  return query.filter(cls.requested_time >= start_date).filter(

[wrengr, 2016/10/05 18:41:58]

Filtering in one pass avoids constructing an intermediate |query|. (Unless you
just so happen to have implemented |cls.query| in a special way that
automatically fuses the repeated passes.)

[Sharu Jiang, 2016/10/06 18:16:56]

Done.

+      cls.requested_time < end_date)
+
+
+def IterateClientCrashes(client_id,
+                         callback_func,

[wrengr, 2016/10/05 18:41:58]

Rather than passing a callback function, why not simply define this function
(and the underlying (crash_iterator.IterateCrashes) as generators? Using
generators leads to much cleaner code, as it avoids callback hell.

[Sharu Jiang, 2016/10/06 18:16:56]

This is a good idea :)

+                         crash_info_fields=None,
+                         property_values=None,
+                         start_date=_A_YEAR_AGO,
+                         end_date=_TODAY,
+                         batch_size=_DEFAULT_BATCH_SIZE,
+                         batch_run=False):
+  """Pulls crashes from client and passes needed info to callback function.
+
+  Args:
+    client_id (CrashClient): One of CrashClient.FRACAS, CrashClient.CRACAS,
+      CrashClient.CLUSTERFUZZ.
+    callback_func (function): A function with a crash_info dict as the
+      single parameter.
+    crash_info_fields (list): Field names of a crash info to pass over to the
+      callback function.
+    property_values (dict): Property values to query.
+    start_date (str): Only iterate testcases after this date including this
+      date, format '%Y-%m-%d'.
+    end_date (str): Only iterate testcases before this date excluding this date,
+      format '%Y-%m-%d'.
+    batch_size (int): The number of crashes to query at one time.
+    batch_run (bool): If True, run callback_func on a batch of crashes, if
+      False, run callback_func on each crash.
+
+    An example is available in crash_printer/print_crash.py.
+  """
+  if crash_info_fields is None:
+    crash_info_fields = GetCrashInfoFields(client_id)
+
+  if property_values is None:
+    property_values = {}
+
+  query = GetQueryForClient(client_id, property_values, start_date, end_date)
+  crash_iterator.IterateCrashes(query, crash_info_fields, callback_func,
+                                batch_size=batch_size, batch_run=batch_run)