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

Select API helper based on presence of grpcio + optional environment marker #1816

Merged
merged 1 commit into from
Jun 14, 2016
Merged

Select API helper based on presence of grpcio + optional environment marker #1816

merged 1 commit into from
Jun 14, 2016

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented May 23, 2016

Not yet ready to merge, as the system tests will break until new versions of grpcio and google-gax-pubusb-v1 are released (see #1814).

@dhermes PTAL at the overall approach to choosing gRPC vs. JSON-over-HTTP.

@tseaver tseaver added do not merge Indicates a pull request not ready for merge, due to either quality or timing. api: pubsub Issues related to the Pub/Sub API. labels May 23, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 23, 2016
@dhermes
Copy link
Contributor

dhermes commented May 23, 2016

I'm fine with the overall approach. As usual, I'm anti-"lazy loading approach". I'd also like to see us try to unify the APIs that use gRPC a little bit (doesn't need to happen here, just want to keep an eye on it).

If present, prefer the GAX helper, but allow disabling it via an
environment marker.
@tseaver tseaver removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 13, 2016
@tseaver
Copy link
Contributor Author

tseaver commented Jun 13, 2016

Issue #1857 tracks the question of lazy-loading the API helpers.

@tseaver
Copy link
Contributor Author

tseaver commented Jun 13, 2016

@daspecster PTAL

@daspecster daspecster mentioned this pull request Jun 13, 2016
7 tasks
@daspecster
Copy link
Contributor

@tseaver LGTM!

@tseaver tseaver merged commit 1ab2169 into googleapis:master Jun 14, 2016
@tseaver tseaver deleted the pubsub-optional_grpc_publisher_api_impl branch June 14, 2016 00:50
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants