Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable gRPC for datastore API. #2099

Merged
merged 2 commits into from
Aug 18, 2016
Merged

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 12, 2016

NOTE: Has #2097 as diffbase.

@dhermes dhermes added api: datastore Issues related to the Datastore API. grpc labels Aug 12, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 12, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Aug 12, 2016

@tseaver I don't have a flag for disabling it, but am happy to add it, just wanted to here from you about it.

from gcloud.datastore._generated.datastore_pb2 import RollbackResponse
from gcloud.datastore._generated.datastore_pb2 import RunQueryRequest
from gcloud.datastore._generated.datastore_pb2 import RunQueryResponse
# END: Manually added imports

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Aug 17, 2016

The environment flags for pubsub and logging were basically there to allow me to test with the gRPC team, without turning the feature on by default. We can probably leave them out here, if you don't feel paranoid about the gRPC stuff breaking us.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

if you don't feel paranoid about the gRPC stuff breaking us

I don't feel paranoid

@tseaver
Copy link
Contributor

tseaver commented Aug 17, 2016

I don't feel paranoid

Hmm.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

Hmm aside, let's get #2097 settled and then move back here. (I'll resolve the merge conflict.)

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

@tseaver Rebased after #2097, PTAL.

The question remaining is how to update make_operations_grpc.py to have all the imports. Should I just import all the names from gcloud.datastore._generated.datastore_pb2?


As for worry about gRPC, I was more saying I'm not worried about people having imports work but wanting to turn it off. And those people can just hack and edit the module-private _HAS_GRPC

@tseaver
Copy link
Contributor

tseaver commented Aug 17, 2016

The question remaining is how to update make_operations_grpc.py to have all the imports. Should I just import all the names from gcloud.datastore._generated.datastore_pb2?

This is the canonical case for from foo import *, even.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

Trying to avoid the import *

@dhermes
Copy link
Contributor Author

dhermes commented Aug 17, 2016

@tseaver Check out the latest commit

from grpc.beta import implementations as beta_implementations
from grpc.beta import interfaces as beta_interfaces
from grpc.framework.common import cardinality
from grpc.framework.interfaces.face import utilities as face_utilities

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Aug 18, 2016

LGTM

@dhermes dhermes merged commit a9891ab into googleapis:master Aug 18, 2016
@dhermes dhermes deleted the datastore-add-grpc branch August 18, 2016 16:25
@dhermes dhermes mentioned this pull request Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. cla: yes This human has signed the Contributor License Agreement. grpc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants