Add cloud datastore implementation.

Add cloud datastore implementation.

Fixes #52.

BUG=

Committed: https://github.com/luci/gae/commit/6caae3281b2245a2a1c2c0ced695ca1ab81605c2

[dnj, 2016-05-08 19:48:43]

dnj@chromium.org changed reviewers:
+ iannucci@chromium.org

[dnj, 2016-05-08 19:48:44]

PTAL. Fun weekend project.

[dnj, 2016-05-08 19:53:21]

Description was changed from

==========
Add cloud datastore implementation.

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

to

==========
Add cloud datastore implementation.

Fixes #52.

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

[dnj, 2016-05-18 18:20:21]

Hmm ping? I actually might use this in my microservices to avoid over-QPS-ing
the LogDog Coordinator.

[iannucci, 2016-05-23 21:53:06]

lgtm

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

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/datastore.go#new...
impl/cloud/datastore.go:53: appID       string
namespace too?

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/datastore.go#new...
impl/cloud/datastore.go:59: // datastore library does not offer this guarantee.
IMO, I would just change the high level AllocateIDs API to follow the cloud
datastore api: you provide some keys and then it fills the IDs in. It's a better
api anyway.

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/datastore.go#new...
impl/cloud/datastore.go:94: // The cloud datastore SDK does not expose any
transaction options.
well that's a bummer.

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/datastore.go#new...
impl/cloud/datastore.go:99: case opts.Attempts != 0 && opts.Attempts != 3:
This is actually a client library responsibility. You should have this
implementation loop appropriately (that's all this option does in the gae SDK
too, it never gets passed to the server).

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/datastore.go#new...
impl/cloud/datastore.go:207: // potentially wasteful in the event o failed or
retried transactions, but
s/o/of

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/info.go
File impl/cloud/info.go (right):

https://codereview.chromium.org/1957953002/diff/1/impl/cloud/info.go#newcode80
impl/cloud/info.go:80: func (i *infoService) VersionID() string               {
return "" }
let's return dummy values and/or panic; I'd rather know early that we're using
an unimplemented/meaningless interface rather than have things silently 'work'
with empty strings.

[dnj (Google), 2016-07-01 02:11:28]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-07-01 02:11:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1957953002/#ps40001
(title: "Rebase")

[dnj (Google), 2016-07-01 02:11:35]

The CQ bit was unchecked by dnj@google.com

[commit-bot: I haz the power, 2016-07-01 02:11:36]

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, 2016-07-01 02:22:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-01 02:22:27]

Try jobs failed on following builders:
  Luci-GAE Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Luci-GAE%20Presubmit/bu...)

[dnj (Google), 2016-07-01 02:25:56]

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/datastore.go
File impl/cloud/datastore.go (right):

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/datastore...
impl/cloud/datastore.go:53: appID       string
On 2016/05/23 21:53:06, iannucci wrote:
> namespace too?

I'm tracking that in Info service and applying it above in the "use" function.

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/datastore...
impl/cloud/datastore.go:59: // datastore library does not offer this guarantee.
On 2016/05/23 21:53:06, iannucci wrote:
> IMO, I would just change the high level AllocateIDs API to follow the cloud
> datastore api: you provide some keys and then it fills the IDs in. It's a
better
> api anyway.

Done.

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/datastore...
impl/cloud/datastore.go:94: // The cloud datastore SDK does not expose any
transaction options.
On 2016/05/23 21:53:06, iannucci wrote:
> well that's a bummer.

Acknowledged.

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/datastore...
impl/cloud/datastore.go:99: case opts.Attempts != 0 && opts.Attempts != 3:
On 2016/05/23 21:53:06, iannucci wrote:
> This is actually a client library responsibility. You should have this
> implementation loop appropriately (that's all this option does in the gae SDK
> too, it never gets passed to the server).

Oh good to know. Implemented in the same manner as the AppEngine client library.

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/datastore...
impl/cloud/datastore.go:207: // potentially wasteful in the event o failed or
retried transactions, but
On 2016/05/23 21:53:06, iannucci wrote:
> s/o/of

Done.

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/info.go
File impl/cloud/info.go (right):

https://chromiumcodereview.appspot.com/1957953002/diff/1/impl/cloud/info.go#n...
impl/cloud/info.go:80: func (i *infoService) VersionID() string               {
return "" }
On 2016/05/23 21:53:06, iannucci wrote:
> let's return dummy values and/or panic; I'd rather know early that we're using
> an unimplemented/meaningless interface rather than have things silently 'work'
> with empty strings.

Done I left FullyQualifiedAppID implemented and blank b/c datastore service
layer requires it. Empty string is fine I think, since it's not forwarded to
cloud SDK layer anyway.

[dnj (Google), 2016-07-01 02:26:02]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-07-01 02:26:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1957953002/#ps60001
(title: "Comments.")

[commit-bot: I haz the power, 2016-07-01 02:26:08]

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, 2016-07-01 02:36:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-01 02:36:56]

Try jobs failed on following builders:
  Luci-GAE Presubmit (Swarming) on master.tryserver.infra (JOB_FAILED,
https://luci-milo.appspot.com/swarming/prod/2fbd18bbc72df110)

[dnj (Google), 2016-07-01 02:37:39]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-07-01 02:37:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1957953002/#ps80001
(title: "Adjust minimum coverage.")

[commit-bot: I haz the power, 2016-07-01 02:37:45]

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, 2016-07-01 02:43:43]

Description was changed from

==========
Add cloud datastore implementation.

Fixes #52.

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

to

==========
Add cloud datastore implementation.

Fixes #52.

BUG=

Committed:
https://github.com/luci/gae/commit/6caae3281b2245a2a1c2c0ced695ca1ab81605c2
==========

[commit-bot: I haz the power, 2016-07-01 02:43:44]

Committed patchset #5 (id:80001) as
https://github.com/luci/gae/commit/6caae3281b2245a2a1c2c0ced695ca1ab81605c2