Skip to content

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
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