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

Removing usage of the gRPC beta subpackage. #2149

Merged
merged 3 commits into from
Aug 22, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 19, 2016

Fixes #2130.

Most noteworthy is that the "stable" gRPC doesn't have __exit__ and __enter__ and that the stubs are actual classes (instead of some instance of a private and esoteric class).

When scanning for the case-insenstive string grpc I also found some things to change in other parts of the codebase I came across (e.g. a virtualenv root path falling back to protoc)


@nathanielmanistaatgoogle PTAL

In particular we went from the public exception type grpc.framework.interfaces.face.face.AbortionError to the private grpc._channel._Rendezvous.

How are we or users supposed to build in error handling? Should we catch grpc.Error (which _Rendezvous inherits from)?

BIG WORRY: Every system tests finishes with

E0819 12:40:27.346506119   22315 network_status_tracker.c:48] Memory leaked as all network endpoints were not shut down

@dhermes dhermes added api: datastore Issues related to the Datastore API. api: pubsub Issues related to the Pub/Sub API. api: bigtable Issues related to the Bigtable API. api: logging Issues related to the Cloud Logging API. grpc labels Aug 19, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 19, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Aug 19, 2016

@dhermes
Copy link
Contributor Author

dhermes commented Aug 19, 2016

@nathanielmanistaatgoogle It's also worth noting that GAX is still using the beta API /cc @bjwatson

I realized that and had to modify google.gax.grpc.exc_to_code to allow for _Rendezvous exceptions

@bjwatson
Copy link

I created googleapis/gax-python#126 to track this.

from grpc.beta import implementations
from google.gax.grpc import exc_to_code as beta_exc_to_code
import grpc
from grpc._channel import _Rendezvous

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Also fixing a test mock failure for gcloud._helpers.
def __exit__(self, exc_type, exc_val, exc_t):
"""Stops the client as a context manager."""
self.stop()

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Aug 22, 2016

Did we ditch the timeouts because the GA API no longer supports them? I like the simplicity on our side, but worry that users might still need them.

@tseaver
Copy link
Contributor

tseaver commented Aug 22, 2016

LGTM

@dhermes
Copy link
Contributor Author

dhermes commented Aug 22, 2016

@nathanielmanistaatgoogle Filed #2156 to discuss our current disagreement on interface. @tseaver needs this PR for some upstream so "let's don't" block him and we can hopefully get more changes in as a resolution of #2156.

@dhermes dhermes merged commit 20eea35 into googleapis:master Aug 22, 2016
@dhermes dhermes deleted the remove-beta-grpc branch August 22, 2016 19:57
@tseaver tseaver mentioned this pull request Aug 25, 2016
@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: bigtable Issues related to the Bigtable API. api: datastore Issues related to the Datastore API. api: logging Issues related to the Cloud Logging API. api: pubsub Issues related to the Pub/Sub 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.

5 participants