[Predator] Add flag manager and flag manager test.

appengine/findit/crash/flag_manager.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.
+
+"""Module handles flags while doing stream parsing."""

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

It'd be helpful to explain what "flags" are. I know we talked about it, but it'd
be good to put in the docstring. Perhaps with a concrete example of some
stacktrace that has them

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+
+from collections import defaultdict
+
+
+class Flag(object):
+  """Represents a flag of a obj.

[wrengr, 2016/12/22 22:31:10]

"a obj" -> "an obj". But (a) we prefer avoiding abbreviations, and (b) it'd be
helpful to be more explicit about what a "flag" actually is, what sorts of
objects might want them, etc.

Also, it'd help to rename this to something like StacktraceParsingFlag since the
word "flag" is pretty generic and doesn't help much in explaining what the
intended purpose is.

[Sharu Jiang, 2016/12/27 23:48:03]

Renamed to ``ParsingFlag`` :)

+
+  This object serves like a delegation to manipulate flag of obj,
+  condition will be used to evaluate the flag value.
+  """
+  def __init__(self, name, condition=None, val=False):

[wrengr, 2016/12/22 22:31:10]

val -> value

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+    """Inits Flag to point to the flag of obj.
+
+    Args:
+      name (str): Name of the flag.
+      condition (function): Funtion that returns bool, the condition to turn on

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

what arguments does the function take? in what sort of contexts is it called?

[Sharu Jiang, 2016/12/27 23:48:04]

Done.

+        the flag when the flag is off.
+      val (bool): Initial value of the flag.
+    """
+    self._name = name
+    self._condition = condition
+    self._val = val
+
+  @property
+  def name(self):
+    return self._name
+
+  @property
+  def val(self):
+    return self._val
+
+  @val.setter
+  def val(self, val):
+    self._val = val

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

Rather than having a setter, I think it'd be better to have methods for
enabling/setting and disabling/clearing. That way it's harder for clients to
accidentally set it to an int or something

[Sharu Jiang, 2016/12/27 23:48:03]

good point, done.

+
+  def __nonzero__(self):
+    return self._val
+
+  __bool__ = __nonzero__
+
+  def ConditionallyTurnOn(self, line):
+    """When the flag is off, turn on it if conditions met."""
+    if not self._val and self._condition and self._condition(line):
+      self._val = True
+
+
+class FlagManager(object):
+  """A manager to manage manipulations of all the registered flags.

[wrengr, 2016/12/22 22:31:10]

"manager to manage" is awkward

[Sharu Jiang, 2016/12/27 23:48:03]

Oops.

+
+  FlagManager groups and manages flags based on group name. FlagManager takes

[wrengr, 2016/12/22 22:31:10]

suggest: "groups" -> "collects"

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+  care of manipulations of flags during the stream parsing, include evaluating

[wrengr, 2016/12/22 22:31:10]

"manipulations of" -> "manipulating"

"include evaluating" -> "including evaluating"

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+  based on condition of the flag and resetting the flags.

[wrengr, 2016/12/22 22:31:10]

"based on condition" -> "based on the condition"

also comma before "and"

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+  """
+
+  def __init__(self):
+    self.flag_groups = defaultdict(list)
+    self.flags = {}
+
+  def ClearFlags(self):
+    self.flag_groups = defaultdict(list)
+    self.flags = {}
+
+  def Register(self, group_name, flag):
+    """Registers a list of flags of a group.

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

list? looks like it only takes one flag

Also the preposition "of" is awkward here. I think "registers a flag with a
group" is probably the most natural way to phrase it.

[Sharu Jiang, 2016/12/27 23:48:04]

Oops, forgot to update the doc str.

+
+    Flags under the same group should have the same scope.
+    For example, in stacktrace parsing, the scope of a flag like
+    ``after_sumary_line`` is the whole stacktrace, so it should be under
+    ``stacktrace_flags`` group, while the scope of
+    ``callstack_top_frame_has_no_symbol`` is callstack, and it should be
+    under ``callstack_flags`` group.
+    """
+    self.flag_groups[group_name].append(flag)
+    self.flags[flag.name] = flag
+
+  def GetFlagGroup(self, group_name=None):
+    """Returns flags of a group or all flags if None group provided."""

[wrengr, 2016/12/22 22:31:10]

"flags of a group" -> "a group's flags"

comma before "or".

"None" -> "no". Reads more naturally; also more accurate, since we look at the
truthiness of group_name rather than ``is None``

Also, I think it'd be clearer to separate this into two different methods: one
for returning all flags, and another for getting the flags of a specific group.
I don't really see the value in combining them into one method.

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+    if group_name:
+      return self.flag_groups.get(group_name, [])
+
+    flags = []
+    for group_flags in self.flag_groups.values():
+      flags.extend(group_flags)
+
+    return flags
+
+  def Reset(self, group_name=None):

[wrengr, 2016/12/22 22:31:10]

docstring. Especially since it does things based on GetFlagGroup rather than
just resetting a single flag

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+    for flag in self.GetFlagGroup(group_name):
+      flag.val = False
+
+  def ConditionallyTurnOn(self, line, group_name=None):

[wrengr, 2016/12/22 22:31:10]

ditto

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+    for flag in self.GetFlagGroup(group_name):
+      flag.ConditionallyTurnOn(line)
+
+  def Get(self, flag_name):

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

ditto

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+    return self.flags.get(flag_name)
+
+  def Set(self, flag_name, val):

[wrengr, 2016/12/22 22:31:10]

ditto

[Sharu Jiang, 2016/12/27 23:48:03]

Done.

+    flag = self.flags.get(flag_name)
+    if flag is None:
+      return
+
+    flag.val = val