-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Re-enable bundling (for consideration) #1950
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
Re-enable bundling (for consideration) #1950
Conversation
gcloud/pubsub/_gax.py
Outdated
| result = self._gax_api.publish(topic_path, message_pbs, | ||
| options=options) | ||
| # result = self._gax_api.publish(topic_path, message_pbs, | ||
| # options=options) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@daspecster Can you try running the $ tox -e system-tests --notest
$ GCLOUD_ENABLE_GAX .tox/system-tests/bin/python \
system_tests/run_system_test.py --package=pubsub |
|
LGTM once Travis passes, assuming the system tests pass for you with gRPC/GAX enabled. |
|
I ran it with this... GCLOUD_TESTS_PROJECT_ID=ferrous-arena GOOGLE_APPLICATION_CREDENTIALS=creds.json GCLOUD_ENABLE_GAX=True .tox/system-tests/bin/python system_tests/run_system_test.py --package=pubsub |
|
LGTM |
|
@bjwatson Ugh, are you saying that the only change we needed was to set |
|
I think so. |
|
@bjwatson, these are the only references to I'm not sure how the discovery stuff works but, what/how would pick up on the |
|
@daspecster The first reference is the one that does the magic: https://github.com/googleapis/gax-python/blob/75fc5bdbbad46ab1d08d30718ef1485ae211eb5b/google/gax/__init__.py#L134 It depends on I'm not sure about Discovery. Are you doing anything with that? It's just generating file-length sample code right now, and not any API surfaces. |
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
I know this is probably up for debate, I just wanted to learn more about this.
If you guys do want to re-enable this(#1911) I'll make whatever changes are needed!
Or let me know and I'll close this if you want to go in a different direction.