Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 3005413002: [dashboard] Remove memcache and _MAX_NUM_PARTS from stored_object. (Closed)

Created:
2 months, 1 week ago by dtu
Modified:
2 months, 1 week ago
Reviewers:
perezju, shatch, sullivan
CC:
catapult-reviews_chromium.org, perf-dashboard-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

[dashboard] Remove memcache and _MAX_NUM_PARTS from stored_object. * Remove memcache. NDB already has both memcache and in-context cache, and calling out to memcache directly bypasses the latter. * Remove _MAX_NUM_PARTS. Without memcache, there's no limit to the size of stored_objects. This also removes a bug where it was always storing and retrieving _MAX_NUM_PARTS PartEntities in the datastore. This is the lite version of https://codereview.chromium.org/3011223003. It has only backwards compatible changes. BUG=catapult:#3834 Review-Url: https://chromiumcodereview.appspot.com/3005413002 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/2070d1f7afd90ab81380dcc5a8d1701e335e020e

Patch Set 1 #

Patch Set 2 : Docstring., #

Total comments: 4

Patch Set 3 : Remove _GetValueFromDatastore. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -155 lines) Patch
M dashboard/dashboard/common/stored_object.py View 1 2 8 chunks +16 lines, -121 lines 0 comments Download
M dashboard/dashboard/common/stored_object_test.py View 4 chunks +0 lines, -34 lines 0 comments Download

Messages

Total messages: 18 (8 generated)
dtu
The lite version. I wanted to keep this minimal, and excluded some other code simplifications ...
2 months, 1 week ago (2017-09-12 22:59:29 UTC) #3
shatch
lgtm % question https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py File dashboard/dashboard/common/stored_object.py (left): https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py#oldcode132 dashboard/dashboard/common/stored_object.py:132: if len(serialized_parts) > _MAX_NUM_PARTS: Hmm should ...
2 months, 1 week ago (2017-09-13 01:18:32 UTC) #4
perezju
lgtm w/another question https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py File dashboard/dashboard/common/stored_object.py (right): https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py#newcode47 dashboard/dashboard/common/stored_object.py:47: results = get_future.get_result() My comment from ...
2 months, 1 week ago (2017-09-13 13:02:10 UTC) #5
perezju
lgtm w/another question https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py File dashboard/dashboard/common/stored_object.py (right): https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py#newcode47 dashboard/dashboard/common/stored_object.py:47: results = get_future.get_result() My comment from ...
2 months, 1 week ago (2017-09-13 13:02:11 UTC) #6
dtu
https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py File dashboard/dashboard/common/stored_object.py (left): https://codereview.chromium.org/3005413002/diff/20001/dashboard/dashboard/common/stored_object.py#oldcode132 dashboard/dashboard/common/stored_object.py:132: if len(serialized_parts) > _MAX_NUM_PARTS: On 2017/09/13 01:18:32, shatch wrote: ...
2 months, 1 week ago (2017-09-13 15:24:06 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/3005413002/20001
2 months, 1 week ago (2017-09-13 15:24:16 UTC) #9
perezju
On 2017/09/13 15:24:16, commit-bot: I haz the power wrote: > CQ is trying da patch. ...
2 months, 1 week ago (2017-09-13 15:26:01 UTC) #10
dtu
On 2017/09/13 15:26:01, perezju wrote: > On 2017/09/13 15:24:16, commit-bot: I haz the power wrote: ...
2 months, 1 week ago (2017-09-13 15:28:15 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/3005413002/40001
2 months, 1 week ago (2017-09-13 15:28:28 UTC) #15
commit-bot: I haz the power
2 months, 1 week ago (2017-09-13 16:52:42 UTC) #18
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld efc10ee0f