Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(37)

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

Created:
3 years, 3 months ago by dtu
Modified:
3 years, 3 months 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 ...
3 years, 3 months 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 ...
3 years, 3 months 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 ...
3 years, 3 months 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 ...
3 years, 3 months 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: ...
3 years, 3 months 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
3 years, 3 months 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. ...
3 years, 3 months 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: ...
3 years, 3 months 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
3 years, 3 months ago (2017-09-13 15:28:28 UTC) #15
commit-bot: I haz the power
3 years, 3 months 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 408576698