Skip to content

Factor out API helper classes from 'pubsub.Connection'. #1736

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.

Sign up for GitHub

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

Jump to bottom
Merged
tseaver merged 8 commits into from
Apr 22, 2016
Merged

Factor out API helper classes from 'pubsub.Connection'. #1736

tseaver merged 8 commits into from
Apr 22, 2016

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Apr 22, 2016 •
edited
Loading

Uses #1700 as a base.

Toward #851 / #1696.

@tseaver tseaver added the api: pubsub Issues related to the Pub/Sub API. label Apr 22, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 22, 2016
@tseaver tseaver mentioned this pull request Apr 22, 2016
Merged
tseaver added 6 commits April 22, 2016 12:14
@tseaver
Add 'Client.{publisher,subscriber,iam_policy}_api' properties.
159d63a 
These API objects will represent logical groupings of the JSON api
endpoints, and correspond one-for-one to the stubs exposed by the
generated gRPC code.

Further commits will move the endpoint-specific methods from the
'Connecttion' class into the relevant API classes, and adjust the
callers.

See:

#1700 (comment)
@tseaver
Move 'list_topics' from 'Connection' to '_PublisherAPI'.
2c7c58b
@tseaver
Move 'list_subscriptions' from 'Connection' to '_SubscriberAPI'.
8edaec6
@tseaver
Move remaining publisher API methods from 'Connection' to '_Publisher…
2a54ffb 
…API'.
@tseaver
Move remaining subscriber API methods from 'Connection' to '_Subscrib…
75bc6de 
…erAPI'.
@tseaver
Move remaining IAM policy API methods from 'Connection' to '_IAMPolic…
080313c 
…yAPI'.
@tseaver
Copy link
Contributor Author

tseaver commented Apr 22, 2016

@dhermes Rebased after merging #1700 to master. PTAL

dhermes
dhermes reviewed Apr 22, 2016
View reviewed changes
gcloud/pubsub/client.py
def publisher_api(self):
"""Helper for publisher-related API calls."""
if self._publisher_api is None:
self._publisher_api = _PublisherAPI(self.connection)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016

For non-test code, ISTM the only change after the cut-and-paste was self.api_request -> self.conn.api_request (and synonyms). Is that correct?

dhermes
dhermes reviewed Apr 22, 2016
View reviewed changes
gcloud/pubsub/test_connection.py
self.assertEqual(conn.build_api_url('/foo', api_base_url=base_url2),
URI)

def _verify_uri(self, uri, expected_path, **expected_qs):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016 •
edited
Loading

@tseaver LGTM save for my two three clarifications requested.

@tseaver
Copy link
Contributor Author

tseaver commented Apr 22, 2016

For non-test code, ISTM the only change after the cut-and-paste was self.api_request -> self.conn.api_request (and synonyms). Is that correct?

For the helper objects, yes. The callers now delegate to the methods of the helpers, rather than the connection.

@tseaver tseaver merged commit d97cffe into googleapis:master Apr 22, 2016
@tseaver tseaver deleted the 851-1696-pubsub-api_helper_classes branch April 22, 2016 18:01
@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016

@tseaver You jumped the gun a bit on the merge. I have an unanswered question:

Why couldn't those lines just be deleted?

regarding setting test connections to object(). Why do they need to be set at all?

@tseaver
Copy link
Contributor Author

tseaver commented Apr 22, 2016

141eab5 removed them.

@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016

I'm seeing

conn = client.connection = object()

on line 36 of gcloud/pubsub/test_client.py. Lines like that are ones I am curious about.

Instead of self.assertTrue(api._connection is conn) you could use self.assertIsNone(api._connection)

@dhermes dhermes mentioned this pull request Apr 22, 2016
Merged
@tseaver tseaver mentioned this pull request May 24, 2016
Merged
parthea added a commit that referenced this pull request Nov 24, 2025
@ohmayr @parthea
build: Update googleapis version (#1736)
d1a4e5b 
Co-authored-by: Anthonios Partheniou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tseaver @dhermes @googlebot