Port Datastore onto GAPIC #2746

bjwatson · 2016-11-17T02:41:46Z

What

Port the gRPC code path for Datastore onto GAPIC. The package is available here.

Why

This is not a beta-blocker, since there already is gRPC code for Datastore, but it is preferred.

dhermes · 2016-11-17T03:07:37Z

@bjwatson Two questions:

Why doesn't the new package depend on google-gax>=0.15 (which has the new auth helpers)?
Are we in a situation where some of the GAPIC packages depend on 0.13 <= google-gax < 0.14 while others depend on 0.14 <= google-gax < 0.15 (as this one does)?

bjwatson · 2016-11-17T14:53:15Z

@dhermes Only because @geigerj says that he needs to make some small updates to the GAPIC generator for it to be compatible with google-gax>=0.15. He might be able to get that done today, and then we can build all packages with the newer google-gax dependency.

I thought I would get this in your hands right away, rather than waiting for the necessary fixes to support the newer auth helpers. However, if you think it's a more efficient use of your time to wait, then we can do that.

dhermes · 2016-11-18T17:57:08Z

@bjwatson Sorry I didn't notice this until now, but the current form of grpc-google-cloud-datastore-v1 can't run without gRPC. This means we can't get access to the protobufs without forcing grpc to be installed.

Right now we support Protobuf-over-HTTP and Protobuf-over-gRPC, so we need access to the protobuf classes in use cases where gRPC isn't available (i.e. App Engine).

The generated protobuf classes currently live in datastore._generated, which is how we import them. In order to support gRPC we have datastore_pb2.py and datastore_grpc_pb2.py in the _generated module

/cc @jonparrott

bjwatson · 2016-11-19T02:30:22Z

@dhermes Does googleapis/api-client-staging@6649c45 do what you need? Do you also need changes to setup.py? I see that your setup.py depends on grpcio regardless.

Note that this will become unnecessary once grpc/grpc#8056 is released in grpcio==1.1.0.

dhermes · 2016-11-21T18:52:16Z

Yes it does do what I had in mind.

We had conditional checks in our setup.py, but @jonparrott discouraged that since it confuses wheel building. But maybe I am wrong on how I remember that.

AFAIK, even though grpcio is in our setup.py for packages like google-cloud-datastore, it doesn't stop from installing into a lib/ dir in an App Engine app. @jonparrott can confirm or deny or throw his hands up in sadness over packaging?

theacodes · 2016-11-21T18:56:11Z

grpcio will indeed be vendored into lib for app engine and explode on import.

dhermes · 2016-11-21T19:41:47Z

@bjwatson Ping me once released?

geigerj · 2016-11-21T19:51:46Z

@bjwatson Is this a Datastore-specific hand-edit, or do we intend to provide these conditional imports across the gRPC packages we maintain? If it's the latter, we should figure out what the timeline for grpcio 1.1.0 is and whether we need to automate this in our tooling.

dhermes · 2016-11-21T19:53:07Z

@geigerj @bjwatson We would love to see a new release of grpcio because of grpc/grpc#7849, so I am on team "pushing for release"

bjwatson · 2016-11-30T11:12:15Z

@dhermes We're going to wait on porting Datastore to GAPIC until after the beta announcement. We discussed and we don't want to take any chances on destabilizing the google-cloud-python code for the four APIs. The only one that will go beta with GAPIC is Logging.

dhermes · 2016-12-02T18:32:35Z

SGTM

Towards googleapis#2746. This approach is to slowly transition from our current approach to use the GAPIC generated surface. It is unfortunately tangled quite a bit (partly because we may have too much mocked in the tests).

Towards googleapis#2746 (as is googleapis#3064). This approach is to slowly transition from our current approach to use the GAPIC generated surface. These unit tests weren't so bad to update. I did "editorialize" as I went, downgrading constants to just variables, renamining for PEP8, etc.

Towards googleapis#2746. This approach is to slowly transition from our current approach to use the GAPIC generated surface. It is unfortunately tangled quite a bit (partly because we may have too much mocked in the tests).

bjwatson · 2017-03-02T16:58:26Z

@lukesneeringer @dhermes If we're blocking the x-goog-api-client header updates on this issue, then it's P0.

dhermes · 2017-03-02T17:33:10Z

@bjwatson Those are already in for HTTP. I can get them in to the gRPC transport before the process is finished (#3066 and #3064 will give you an idea of what the process looks like).

bjwatson · 2017-03-02T19:39:17Z

Ok, thanks Danny!

This is to prepare for a removal of the Connection class (relates to googleapis#2746 and googleapis#3105).

This is progress towards googleapis#2746 and makes Connection() act as a proxy for the contents of Client (as intended).

lukesneeringer · 2017-03-17T17:48:28Z

@dhermes This is now done, question mark?

dhermes · 2017-03-17T18:00:22Z

Almost. One follow-up PR, coming in hot.

Fixes googleapis#2746.

bjwatson · 2017-03-17T20:10:54Z

Woo hoo! Great job, Danny!

@lukesneeringer @dhermes Does this mean that we'll have the new HTTP headers working for Datastore as soon as this is published?

lukesneeringer · 2017-03-17T21:50:06Z

I believe we have the HTTP headers now. But, yes.
I am thinking a quick umbrella release next week as part of the GA effort, but we can push a 0.23.x for this if we want it sooner.

dhermes · 2017-03-17T23:12:24Z

I just did a

$ git diff datastore-0.23.0 -- datastore/google

and the only change that is in the public interface is the addition of use_gax in the Client constructor.

lukesneeringer · 2017-03-17T23:25:26Z

Sure, but it is a huge under-the-hood change, because those with GRPC support are suddenly using the GRPC transport now.

dhermes · 2017-03-17T23:37:09Z

@lukesneeringer We already had the gRPC transport (for many releases), it was just the raw stub produced by the protoc plugin rather than the DatastoreClient which wraps that very same class.

lukesneeringer · 2017-03-20T14:29:05Z

Okay.

Towards googleapis#2746 (as is googleapis#3064). This approach is to slowly transition from our current approach to use the GAPIC generated surface. These unit tests weren't so bad to update. I did "editorialize" as I went, downgrading constants to just variables, renamining for PEP8, etc.

Towards googleapis#2746. This approach is to slowly transition from our current approach to use the GAPIC generated surface. It is unfortunately tangled quite a bit (partly because we may have too much mocked in the tests).

This is to prepare for a removal of the Connection class (relates to googleapis#2746 and googleapis#3105).

This is progress towards googleapis#2746 and makes Connection() act as a proxy for the contents of Client (as intended).

Fixes googleapis#2746.

bjwatson added api: datastore Issues related to the Datastore API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Nov 17, 2016

bjwatson assigned dhermes Nov 17, 2016

bjwatson mentioned this issue Nov 19, 2016

Regenerate Datastore and Language gRPC and GAPIC 0.14.x packages googleapis/api-client-staging#106

Merged

bjwatson mentioned this issue Feb 22, 2017

GAX headers #3049

Closed

7 tasks

dhermes mentioned this issue Feb 23, 2017

Isolating Connection objects in _http modules (where relevant). #3057

Merged

dhermes mentioned this issue Feb 23, 2017

Making datastore Connection.commit() return low-level protobuf. #3064

Merged

dhermes mentioned this issue Feb 24, 2017

Making datastore Connection.run_query() return low-level protobuf. #3066

Merged

lukesneeringer added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Feb 28, 2017

This was referenced Mar 3, 2017

Making datastore Connection.begin_transaction() return low-level protobuf. #3091

Merged

Making datastore Connection.rollback() return low-level protobuf. #3092

Merged

Making datastore Connection.allocate_ids() return low-level protobuf. #3093

Merged

dhermes mentioned this issue Mar 6, 2017

Add basic helpers needed for GAPIC client in datastore. #3105

Merged

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Mar 6, 2017

Moving _request and _rpc off the datastore HTTP helper.

1b983d0

This is to prepare for a removal of the Connection class (relates to googleapis#2746 and googleapis#3105).

dhermes mentioned this issue Mar 6, 2017

Moving _request and _rpc off the datastore HTTP helper. #3107

Merged

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Mar 6, 2017

Moving datastore host configuration into Client.

d47957e

This is progress towards googleapis#2746 and makes Connection() act as a proxy for the contents of Client (as intended).

dhermes mentioned this issue Mar 6, 2017

Moving datastore host configuration into Client. #3112

Merged

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Mar 6, 2017

Moving datastore host configuration into Client.

2b54910

This is progress towards googleapis#2746 and makes Connection() act as a proxy for the contents of Client (as intended).

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Mar 6, 2017

Moving datastore host configuration into Client.

ebea954

This is progress towards googleapis#2746 and makes Connection() act as a proxy for the contents of Client (as intended).

This was referenced Mar 8, 2017

Using GAPIC datastore object (and an HTTP equivalent) for allocate_ids. #3121

Merged

Using GAPIC datastore object (and an HTTP equivalent) for rollback. #3126

Merged

Using GAPIC datastore object (and an HTTP equivalent) for begin_transaction. #3134

Merged

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Mar 17, 2017

Restoring datastore Gax exception re-mapping.

8fce7c5

Fixes googleapis#2746.

dhermes mentioned this issue Mar 17, 2017

Restoring datastore Gax exception re-mapping. #3163

Merged

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Mar 17, 2017

Restoring datastore Gax exception re-mapping.

bbeab3d

Fixes googleapis#2746.

dhermes closed this as completed in #3163 Mar 17, 2017

dhermes mentioned this issue Mar 17, 2017

Removing unused code-path for remapping raw gRPC exceptions in datastore. #3164

Merged

richkadel pushed a commit to richkadel/google-cloud-python that referenced this issue May 6, 2017

Moving _request and _rpc off the datastore HTTP helper.

9cdec15

This is to prepare for a removal of the Connection class (relates to googleapis#2746 and googleapis#3105).

richkadel pushed a commit to richkadel/google-cloud-python that referenced this issue May 6, 2017

Moving datastore host configuration into Client.

869f4d6

This is progress towards googleapis#2746 and makes Connection() act as a proxy for the contents of Client (as intended).

richkadel pushed a commit to richkadel/google-cloud-python that referenced this issue May 6, 2017

Restoring datastore Gax exception re-mapping.

72e5c59

Fixes googleapis#2746.

theacodes unassigned dhermes Sep 28, 2018

JustinBeckwith assigned dhermes Feb 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port Datastore onto GAPIC #2746

Port Datastore onto GAPIC #2746

bjwatson commented Nov 17, 2016

dhermes commented Nov 17, 2016

bjwatson commented Nov 17, 2016

dhermes commented Nov 18, 2016

bjwatson commented Nov 19, 2016

dhermes commented Nov 21, 2016

theacodes commented Nov 21, 2016

dhermes commented Nov 21, 2016

geigerj commented Nov 21, 2016

dhermes commented Nov 21, 2016

bjwatson commented Nov 30, 2016

dhermes commented Dec 2, 2016

bjwatson commented Mar 2, 2017

dhermes commented Mar 2, 2017

bjwatson commented Mar 2, 2017

lukesneeringer commented Mar 17, 2017

dhermes commented Mar 17, 2017

bjwatson commented Mar 17, 2017

lukesneeringer commented Mar 17, 2017

dhermes commented Mar 17, 2017

lukesneeringer commented Mar 17, 2017

dhermes commented Mar 17, 2017

lukesneeringer commented Mar 20, 2017

Port Datastore onto GAPIC #2746

Port Datastore onto GAPIC #2746

Comments

bjwatson commented Nov 17, 2016

What

Why

dhermes commented Nov 17, 2016

bjwatson commented Nov 17, 2016

dhermes commented Nov 18, 2016

bjwatson commented Nov 19, 2016

dhermes commented Nov 21, 2016

theacodes commented Nov 21, 2016

dhermes commented Nov 21, 2016

geigerj commented Nov 21, 2016

dhermes commented Nov 21, 2016

bjwatson commented Nov 30, 2016

dhermes commented Dec 2, 2016

bjwatson commented Mar 2, 2017

dhermes commented Mar 2, 2017

bjwatson commented Mar 2, 2017

lukesneeringer commented Mar 17, 2017

dhermes commented Mar 17, 2017

bjwatson commented Mar 17, 2017

lukesneeringer commented Mar 17, 2017

dhermes commented Mar 17, 2017

lukesneeringer commented Mar 17, 2017

dhermes commented Mar 17, 2017

lukesneeringer commented Mar 20, 2017