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 most (direct) connection usage in Pub / Sub #2874

Merged
merged 3 commits into from
Dec 16, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Dec 15, 2016

NOTE: Has #2870 as diffbase.

@dhermes dhermes added hygiene api: pubsub Issues related to the Pub/Sub API. labels Dec 15, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 15, 2016
patch = mock.patch.multiple(
'google.cloud.pubsub._gax',
PublisherClient=mock_publisher_api,
insecure_channel=mock_insecure_channel)
with patch:
result = self._call_fut(connection)
result = self._call_fut(None, host=host, secure=False)

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Dec 15, 2016

@dhermes I'm OK to merge without the change I suggested, if you don't think it would be an improvement.

@dhermes dhermes force-pushed the gax-ignore-connection branch from 201fce2 to c8a0d6e Compare December 15, 2016 20:33
@dhermes
Copy link
Contributor Author

dhermes commented Dec 16, 2016

@tseaver WDYT of my most recent comment above

@tseaver
Copy link
Contributor

tseaver commented Dec 16, 2016

@dhermes It looks to me as though host unused in the non-emulated case. If so, then the call site becomes:

if self.conn.in_emul:
    channel = make_foo(host=emulator_host)
else:
    channel = make_foo(credentials=credentials)

Still using a method of a connection object, but this way
it can be more easily swapped out for a function defined
in that module doing the same task.
@dhermes dhermes force-pushed the gax-ignore-connection branch from c8a0d6e to 80413c5 Compare December 16, 2016 06:27
@dhermes dhermes force-pushed the gax-ignore-connection branch from 80413c5 to a46184c Compare December 16, 2016 06:37
@dhermes
Copy link
Contributor Author

dhermes commented Dec 16, 2016

@tseaver PTAL

@tseaver
Copy link
Contributor

tseaver commented Dec 16, 2016

LGTM

@dhermes dhermes merged commit 3d354dc into googleapis:master Dec 16, 2016
@dhermes dhermes deleted the gax-ignore-connection branch December 16, 2016 18:18
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
Removing most (direct) connection usage in Pub / Sub
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.

4 participants