Add API to allow you to get the non-transactional datastore or taskqueue.

Add API to allow you to get the non-transactional datastore or taskqueue.

R=dnj@chromium.org, estaab@chromium.org, vadimsh@chromium.org
BUG=

Committed: https://github.com/luci/gae/commit/da451b882a62b7fab2f6643c9f1c3e28c89b9680

[iannucci, 2015-12-04 03:09:53]

On 2015/12/03 at 17:15:29, iannucci wrote:
> 

ping :)

[Vadim Sh., 2015-12-04 03:22:04]

i don't fully understand it, but lgtm if it works

https://codereview.chromium.org/1494223002/diff/1/impl/prod/context.go
File impl/prod/context.go (right):

https://codereview.chromium.org/1494223002/diff/1/impl/prod/context.go#newcode28
impl/prod/context.go:28: // AEContextNoTX retrieves the raw
"google.golang.org/appengine" compatible Context.
document the difference between AEContextNoTX and AEContext, it's not obvious

https://codereview.chromium.org/1494223002/diff/1/impl/prod/taskqueue.go
File impl/prod/taskqueue.go (right):

https://codereview.chromium.org/1494223002/diff/1/impl/prod/taskqueue.go#newc...
impl/prod/taskqueue.go:24: return tqImpl{AEContextNoTX(ci)}
why this is so much simpler than the others (or, why others so complicated?)

[iannucci, 2015-12-04 03:23:22]

On 2015/12/04 at 03:22:04, vadimsh wrote:
> i don't fully understand it, but lgtm if it works
> 
> https://codereview.chromium.org/1494223002/diff/1/impl/prod/context.go
> File impl/prod/context.go (right):
> 
>
https://codereview.chromium.org/1494223002/diff/1/impl/prod/context.go#newcode28
> impl/prod/context.go:28: // AEContextNoTX retrieves the raw
"google.golang.org/appengine" compatible Context.
> document the difference between AEContextNoTX and AEContext, it's not obvious
> 
> https://codereview.chromium.org/1494223002/diff/1/impl/prod/taskqueue.go
> File impl/prod/taskqueue.go (right):
> 
>
https://codereview.chromium.org/1494223002/diff/1/impl/prod/taskqueue.go#newc...
> impl/prod/taskqueue.go:24: return tqImpl{AEContextNoTX(ci)}
> why this is so much simpler than the others (or, why others so complicated?)

Ah, I added comments on the previous patchset which may help explain. Basically
the datastore one is complicated because RunInTransaction needs to behave
correctly (i.e. preserve namespace and all user goodies, but actually create a
new transaction instead of trying to hop on top of the old one).

[Vadim Sh., 2015-12-04 03:33:08]

On 2015/12/04 03:23:22, iannucci wrote:
> On 2015/12/04 at 03:22:04, vadimsh wrote:
> > i don't fully understand it, but lgtm if it works
> > 
> > https://codereview.chromium.org/1494223002/diff/1/impl/prod/context.go
> > File impl/prod/context.go (right):
> > 
> >
>
https://codereview.chromium.org/1494223002/diff/1/impl/prod/context.go#newcode28
> > impl/prod/context.go:28: // AEContextNoTX retrieves the raw
> "google.golang.org/appengine" compatible Context.
> > document the difference between AEContextNoTX and AEContext, it's not
obvious
> > 
> > https://codereview.chromium.org/1494223002/diff/1/impl/prod/taskqueue.go
> > File impl/prod/taskqueue.go (right):
> > 
> >
>
https://codereview.chromium.org/1494223002/diff/1/impl/prod/taskqueue.go#newc...
> > impl/prod/taskqueue.go:24: return tqImpl{AEContextNoTX(ci)}
> > why this is so much simpler than the others (or, why others so complicated?)
> 
> Ah, I added comments on the previous patchset which may help explain.

There's only one patchset...

[iannucci, 2015-12-04 03:35:10]

https://codereview.chromium.org/1494223002/diff/1/impl/memory/context.go
File impl/memory/context.go (right):

https://codereview.chromium.org/1494223002/diff/1/impl/memory/context.go#newc...
impl/memory/context.go:28: var _ memContextObj = (*memContext)(nil)
changed to a pointer to be == comparable

https://codereview.chromium.org/1494223002/diff/1/impl/memory/datastore.go
File impl/memory/datastore.go (right):

https://codereview.chromium.org/1494223002/diff/1/impl/memory/datastore.go#ne...
impl/memory/datastore.go:38: ic = context.WithValue(ic, memContextKey,
maybeTxnCtx)
this is necessary so that .RunInTransaction will fork off of the root context
instead of trying to fork off of the already-transactional context.

https://codereview.chromium.org/1494223002/diff/1/impl/memory/taskqueue.go
File impl/memory/taskqueue.go (right):

https://codereview.chromium.org/1494223002/diff/1/impl/memory/taskqueue.go#ne...
impl/memory/taskqueue.go:32: return &taskqueueImpl{x, ic, ns}
don't need to tweak the context because taskqueue only uses it for clock and
mathrand (it never gets passed back out to the user)

[iannucci, 2015-12-04 03:35:21]

(it helps when you publish the comments >_<)

[Vadim Sh., 2015-12-04 03:37:57]

On 2015/12/04 03:35:21, iannucci wrote:
> (it helps when you publish the comments >_<)

My verdicts stands. It still looks cryptic to me, but lgtm it works (and looks
like it is). You can wait for someone else to review, your choice :)

[iannucci, 2015-12-04 03:48:48]

On 2015/12/04 at 03:37:57, vadimsh wrote:
> On 2015/12/04 03:35:21, iannucci wrote:
> > (it helps when you publish the comments >_<)
> 
> My verdicts stands. It still looks cryptic to me, but lgtm it works (and looks
like it is). You can wait for someone else to review, your choice :)

(ah, that's why I thought I had two patches... I forgot to upload the second one
>_<)

[iannucci, 2015-12-04 03:49:51]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2015-12-04 03:49:51]

The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1494223002/#ps20001
(title: "fix doc and naming")

[commit-bot: I haz the power, 2015-12-04 03:49:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1494223002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1494223002/20001

[iannucci, 2015-12-04 03:50:38]

If others are interested in this, feel free to take a look, but we all have
enough code and reviews right now that I don't think the load is worth it. If
anyone wants an in-depth on this, I'd be happy to do that :)

[commit-bot: I haz the power, 2015-12-04 03:52:41]

Description was changed from

==========
Add API to allow you to get the non-transactional datastore or taskqueue.

R=dnj@chromium.org, estaab@chromium.org, vadimsh@chromium.org
BUG=
==========

to

==========
Add API to allow you to get the non-transactional datastore or taskqueue.

R=dnj@chromium.org, estaab@chromium.org, vadimsh@chromium.org
BUG=

Committed:
https://github.com/luci/gae/commit/da451b882a62b7fab2f6643c9f1c3e28c89b9680
==========

[commit-bot: I haz the power, 2015-12-04 03:52:41]

Committed patchset #2 (id:20001) as
https://github.com/luci/gae/commit/da451b882a62b7fab2f6643c9f1c3e28c89b9680