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

Pubsub API redesign broke support for PUBSUB_EMULATOR_HOST #3886

Closed
bpicolo opened this issue Aug 27, 2017 · 16 comments
Closed

Pubsub API redesign broke support for PUBSUB_EMULATOR_HOST #3886

bpicolo opened this issue Aug 27, 2017 · 16 comments
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API.

Comments

@bpicolo
Copy link

bpicolo commented Aug 27, 2017

Broke 2 days ago with this diff:
4a8e155#diff-c15e14c96ae7c1e14ebdff735c3080e6L62

@bpicolo bpicolo changed the title Pubsub API redesign doesn't support PUBSUB_EMULATOR_HOST Pubsub API redesign broke support for PUBSUB_EMULATOR_HOST Aug 27, 2017
@lukesneeringer
Copy link
Contributor

Hi @bpicolo,
Thanks for letting me know.

I admit unfamiliarity with the Pub/Sub emulator. I will look into what I need to do to make this work for you. If it is just an alternative hostname, that will work, but the incantation is slightly different.

@dhermes
Copy link
Contributor

dhermes commented Aug 28, 2017

@lukesneeringer Let's chat about this. I had great support for Pub/Sub, Bigtable and Datastore, but it got nuked when we switched from tox to nox.

@bpicolo
Copy link
Author

bpicolo commented Aug 28, 2017

@lukesneeringer Should just be the hostname I would think (maybe https cert checking and the like as well, unsure what the actual code path looks like)
Seems worthwhile to add some sort of acceptance tests for this?

@lukesneeringer
Copy link
Contributor

@bpicolo Probably.:-)

If it is just hostname, you can send the service_path keyword argument to the client (it gets passed to that object if added in **kwargs here.

@dhermes
Copy link
Contributor

dhermes commented Aug 28, 2017

We previously did not have any CI testing for the emulator integration, but now that we control the Docker image for CircleCI, it would be very easy to add the gcloud SDK (where the emulators live) to the Docker image. Yay!

@hannesholst
Copy link

hannesholst commented Aug 29, 2017

@lukesneeringer & @dhermes Is there an ETA for a fix? The workaround with the **kwargs doesn't work since there's still some https stuff failing alongside it...

@wscheep
Copy link

wscheep commented Sep 4, 2017

Same problem here, hoping for a quick fix as we're using the emulator to test most of our services..

@cgthayer
Copy link

:-( uhhh, how were the API changes tested before the release if the emulator broke #test-in-prod

@robhaswell
Copy link

(This issue is a blocker for me deploying this wonderful new client 😢)

@dhermes

@lukesneeringer Let's chat about this. I had great support for Pub/Sub, Bigtable and Datastore, but it got nuked when we switched from tox to nox.

Thank you for your work on emulator support, however for Pub/Sub there was a noticeable drawback, which was that the client library required any valid GCP service account credentials before it would proceed to use the emulator. Hopefully the replaced emulator support won't have this drawback and will use the emulator without credentials, as e.g. the Datastore client does.

@tseaver tseaver added the api: pubsub Issues related to the Pub/Sub API. label Sep 22, 2017
@robhaswell
Copy link

Hi team, is this issue on the work schedule at all? This part of our system needs some refactoring and I'm trying to work out if we should delay so we can upgrade to the new client. Thanks.

@suhaasprasad
Copy link

Any workaround here? Passing in the service_path and service_port to the client doesn't seem to work and prints the following error:

E1025 00:04:54.085080000 140737010254784 ssl_transport_security.c:940] Handshake failed with fatal error SSL_ERROR_SSL: error:100000f7:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER.

@telenieko
Copy link

telenieko commented Oct 25, 2017

Hi,

According to Google documentation the developer (users of the library) should not have to do anything in code to use the emulator. Only set the proper environment variable. Also that is the way it worked before this ticket (apparently).

There are two things missing IMHO: 1) a test case that checks that when the PUBSUB_EMULATOR_HOST is set all requests to PUBSUB are forwarded there, and 2) support inside both the subscriber and the publisher to set service_path and service_port accordingly at runtime without user intervention.

Also as per @suhaasprasad comment, the client should allow for non-SSL connections when honouring the EMULATOR variable.

Am I missing something?

@telenieko
Copy link

Temporary workaround (ugly) solves the SSL problem:

        if os.environ.get('PUBSUB_EMULATOR_HOST'):
            grpc_channel = grpc.insecure_channel(os.environ['PUBSUB_EMULATOR_HOST'])
            publisher = pubsub_v1.PublisherClient(channel=grpc_channel)
        else:
            publisher = pubsub_v1.PublisherClient()

@suhaasprasad
Copy link

I'm still unclear as to the root issue, but I've reverted to using version 0.27.0 of the google-cloud-pubsub library for now.

@lukesneeringer
Copy link
Contributor

I just merged a fix for this; release should be soon (sometime this week, hopefully in the early half).

@robhaswell
Copy link

Amazing, thanks @lukesneeringer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API.
Projects
None yet
Development

No branches or pull requests

10 participants