[Culprit-Finder] Add generator cache decorator.

[Culprit-Finder] Add generator cache decorator.

BUG=658027
Review-Url: https://codereview.chromium.org/2644543006
Committed: https://chromium.googlesource.com/infra/infra/+/64510db7b3ad9734238bcce7350ca47bced996e1

[Sharu Jiang, 2017-01-20 22:36:15]

Patchset #1 (id:1) has been deleted

[Sharu Jiang, 2017-01-20 22:38:13]

Patchset #1 (id:20001) has been deleted

[Sharu Jiang, 2017-01-20 22:39:10]

Description was changed from

==========
[Culprit-Finder] Add generator cache decorator.

BUG=
==========

to

==========
[Culprit-Finder] Add generator cache decorator.

BUG=658027
==========

[Sharu Jiang, 2017-01-20 22:39:10]

katesonia@chromium.org changed reviewers:
+ chanli@chromium.org, lijeffrey@chromium.org, mbarbella@chromium.org

[Sharu Jiang, 2017-01-20 22:39:56]

PTAL.

[Sharu Jiang, 2017-01-21 00:11:55]

Patchset #1 (id:40001) has been deleted

[Sharu Jiang, 2017-01-21 00:52:58]

Patchset #3 (id:100001) has been deleted

[chanli, 2017-01-21 07:11:56]

https://codereview.chromium.org/2644543006/diff/60001/appengine/findit/libs/c...
File appengine/findit/libs/cache_decorator.py (right):

https://codereview.chromium.org/2644543006/diff/60001/appengine/findit/libs/c...
appengine/findit/libs/cache_decorator.py:112: args (dict): The keyword arguments
of the decorated function.
Nit: kwargs

https://codereview.chromium.org/2644543006/diff/60001/appengine/findit/libs/c...
appengine/findit/libs/cache_decorator.py:137: args (dict): The keyword arguments
of the decorated function.
Nit: kwargs

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
File appengine/findit/libs/cache_decorator.py (right):

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/cache_decorator.py:221: if not cache_success:
Based on docstring, if not cache_success, the loop can be broken right now and
no result would be cached, right? I mean shouldn't you use break instead of
continue here?

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/cache_decorator.py:227: continue
Same here, break instead of continue

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
File appengine/findit/libs/test/cache_decorator_test.py (right):

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/test/cache_decorator_test.py:48: def Func(id1, id2,
url=None):  # Unused parameters-pylint: disable=W0613
nit: Func -> func?

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/test/cache_decorator_test.py:64: namespace=namespace)
Nit: move all parameters in one line?

[Sharu Jiang, 2017-01-23 19:15:35]

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
File appengine/findit/libs/cache_decorator.py (right):

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/cache_decorator.py:221: if not cache_success:
On 2017/01/21 07:11:56, chanli wrote:
> Based on docstring, if not cache_success, the loop can be broken right now and
> no result would be cached, right? I mean shouldn't you use break instead of
> continue here?

In this case, we still need to execute ``yield result``, which means the
``Wrapped`` will generate all results and behave exactly as the decorated
function.

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/cache_decorator.py:227: continue
On 2017/01/21 07:11:56, chanli wrote:
> Same here, break instead of continue

Same as above.

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
File appengine/findit/libs/test/cache_decorator_test.py (right):

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/test/cache_decorator_test.py:48: def Func(id1, id2,
url=None):  # Unused parameters-pylint: disable=W0613
On 2017/01/21 07:11:56, chanli wrote:
> nit: Func -> func?

Done.

https://codereview.chromium.org/2644543006/diff/120001/appengine/findit/libs/...
appengine/findit/libs/test/cache_decorator_test.py:64: namespace=namespace)
On 2017/01/21 07:11:56, chanli wrote:
> Nit: move all parameters in one line?

Done.

[chanli, 2017-01-23 21:37:38]

lgtm

[Sharu Jiang, 2017-01-24 02:04:46]

Patchset #5 (id:160001) has been deleted

[Sharu Jiang, 2017-01-24 02:04:55]

The CQ bit was checked by katesonia@chromium.org

[Sharu Jiang, 2017-01-24 02:04:55]

The patchset sent to the CQ was uploaded after l-g-t-m from chanli@chromium.org
Link to the patchset: https://codereview.chromium.org/2644543006/#ps180001
(title: "Rebase and fix nits.")

[commit-bot: I haz the power, 2017-01-24 02:05:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 02:13:56]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1485223495335380,
"parent_rev": "aa5fe944402e6103032c9232a64abf4def29fc90", "commit_rev":
"64510db7b3ad9734238bcce7350ca47bced996e1"}

[commit-bot: I haz the power, 2017-01-24 02:14:00]

Description was changed from

==========
[Culprit-Finder] Add generator cache decorator.

BUG=658027
==========

to

==========
[Culprit-Finder] Add generator cache decorator.

BUG=658027

Review-Url: https://codereview.chromium.org/2644543006
Committed:
https://chromium.googlesource.com/infra/infra/+/64510db7b3ad9734238bcce7350ca...
==========

[commit-bot: I haz the power, 2017-01-24 02:14:01]

Committed patchset #5 (id:180001) as
https://chromium.googlesource.com/infra/infra/+/64510db7b3ad9734238bcce7350ca...