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

Issue 1222983002: Add filters for RawDatastore (Closed)

Created:
5 years, 5 months ago by iannucci
Modified:
5 years, 5 months ago
Reviewers:
dnj, Vadim Sh., estaab, martiniss
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/infra/infra.git@abstract
Target Ref:
refs/heads/master
Project:
infra
Visibility:
Public.

Description

Add filters for gae library. Filters allow users to add their own wrappers around the underlying gae implementation. They can do anything that an implementation could do, except that their backend is another gae library interface, instead of the SDK or memory implementations. Filters are also necessary (instead of just stacking implementations with SetXXFactory functions), due to transactions, which change the context object. They will be used to implement (at least): * caching (hitting memcache before datastore) * transaction buffering (giving a self-consistent view of the datastore within a transaction, buffering all writes to the end of the transaction to make transaction sizes smaller, etc.) * featureBreaker (for tests, breaking certain APIs with certain errors) * counter (for tests, counting the number of times an API was hit) It may also be used for: * ACL enforcement (rejecting access to datastore items the user can't get, filtering queries, etc.) Thi CL also implements a counter filter as a demo (which will also be used for testing other filter implementations to ensure that the filter being tested). R=dnj@chromium.org, martiniss@chromium.org, vadimsh@chromium.org BUG= Committed: https://chromium.googlesource.com/infra/infra/+/ce7d584c71c3866963c8f12baa95032e092e2f3b

Patch Set 1 #

Patch Set 2 : rename to rdscount #

Patch Set 3 : implementations for all services #

Patch Set 4 : don't need datastoreKey #

Patch Set 5 : fix tests #

Patch Set 6 : tq too #

Patch Set 7 : add example #

Patch Set 8 : golint #

Patch Set 9 : fix some typobugs #

Patch Set 10 : rebase #

Patch Set 11 : rebase #

Patch Set 12 : rebase #

Patch Set 13 : update doc.go #

Patch Set 14 : more doc fixes #

Patch Set 15 : Remove filters/count to simplify CL #

Patch Set 16 : move taskqueue simplification out #

Patch Set 17 : rebase #

Patch Set 18 : rebase #

Patch Set 19 : add tests #

Patch Set 20 : fix tabs #

Patch Set 21 : remove memory diff #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+339 lines, -59 lines) Patch
M go/src/infra/gae/libs/gae/context.go View 1 2 3 4 5 1 chunk +9 lines, -8 lines 0 comments Download
M go/src/infra/gae/libs/gae/doc.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +66 lines, -41 lines 0 comments Download
M go/src/infra/gae/libs/gae/gae.infra_testing View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +2 lines, -2 lines 0 comments Download
M go/src/infra/gae/libs/gae/globalinfo.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +40 lines, -2 lines 2 comments Download
M go/src/infra/gae/libs/gae/memcache.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +40 lines, -2 lines 0 comments Download
M go/src/infra/gae/libs/gae/raw_datastore.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +40 lines, -2 lines 0 comments Download
A go/src/infra/gae/libs/gae/service_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +102 lines, -0 lines 0 comments Download
M go/src/infra/gae/libs/gae/taskqueue.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +40 lines, -2 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 17 (3 generated)
iannucci
5 years, 5 months ago (2015-07-08 06:40:45 UTC) #1
Vadim Sh.
can you please explain what this CL does exactly and why you are doing it?
5 years, 5 months ago (2015-07-08 14:15:39 UTC) #2
iannucci
On 2015/07/08 14:15:39, Vadim Sh. wrote: > can you please explain what this CL does ...
5 years, 5 months ago (2015-07-13 23:11:51 UTC) #3
iannucci
updated doc.go to hopefully make it clearer too (also doc.go was just plain out of ...
5 years, 5 months ago (2015-07-14 23:37:46 UTC) #4
iannucci
+estaab any takers? :)
5 years, 5 months ago (2015-07-16 22:51:29 UTC) #6
estaab
On 2015/07/16 22:51:29, iannucci wrote: > +estaab > > any takers? :) I'll have a ...
5 years, 5 months ago (2015-07-16 22:53:26 UTC) #7
iannucci
FWIW, I moved filters/count into another CL
5 years, 5 months ago (2015-07-16 22:57:11 UTC) #9
martiniss
On 2015/07/16 at 22:57:11, iannucci wrote: > FWIW, I moved filters/count into another CL I'll ...
5 years, 5 months ago (2015-07-16 22:58:09 UTC) #10
iannucci
Actually hold off, I'm going to split this up further.
5 years, 5 months ago (2015-07-16 23:00:47 UTC) #11
iannucci
OK, PTAL now, it's pretty pure and much shorter at this point. As a bonus ...
5 years, 5 months ago (2015-07-16 23:33:18 UTC) #12
Vadim Sh.
lgtm thanks for splitting it in multiple CLs https://codereview.chromium.org/1222983002/diff/400001/go/src/infra/gae/libs/gae/globalinfo.go File go/src/infra/gae/libs/gae/globalinfo.go (right): https://codereview.chromium.org/1222983002/diff/400001/go/src/infra/gae/libs/gae/globalinfo.go#newcode64 go/src/infra/gae/libs/gae/globalinfo.go:64: return ...
5 years, 5 months ago (2015-07-16 23:39:51 UTC) #13
iannucci
thanks for the review, sorry for starting with such a sloppy CL :) https://chromiumcodereview.appspot.com/1222983002/diff/400001/go/src/infra/gae/libs/gae/globalinfo.go File ...
5 years, 5 months ago (2015-07-16 23:46:19 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1222983002/400001
5 years, 5 months ago (2015-07-16 23:46:54 UTC) #16
commit-bot: I haz the power
5 years, 5 months ago (2015-07-16 23:50:26 UTC) #17
Message was sent while issue was closed.
Committed patchset #21 (id:400001) as
https://chromium.googlesource.com/infra/infra/+/ce7d584c71c3866963c8f12baa950...

Powered by Google App Engine
This is Rietveld 408576698