Skip to content

Attempt at implementing clients for Pub/Sub #910

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
merged 15 commits into from
Jun 17, 2015
Merged

Attempt at implementing clients for Pub/Sub #910

merged 15 commits into from
Jun 17, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jun 6, 2015

  • It doesn't seem that in pub/sub there was ever a use of _LocalStack so the worries about the welded on client and batching don't seem to manifest in the pubsub subpackage.
  • I tried to make the commits as small and reviewable as possible.
  • I'm happy to split this up into many PRs to make review / commenting easier.
  • The Using client on all Topic methods. and Using client on all Subscription methods. commits are a bit longer than the rest. I'm also happy to break them up method-by-method if it makes review easier.
  • I didn't look too closely, but there may be some superfluous connection vs. client tests that exist as artifacts of the old / transition states of the code.

dhermes added 12 commits June 5, 2015 15:25
@dhermes
Adding basic client class in pubsub.
4bf5c05 
Allows implicit determination of project and credentials
if no values are passed.
@dhermes
Adding factory constructors to pubsub client.
12460a9
@dhermes
Implementing Client.list_topics in pubsub.
f058b3e
@dhermes
Implementing Client.list_subscriptions in pubsub.
47cc25a
@dhermes
Allowing client to be used in pubsub.Topic.
be42839
@dhermes
Removing gcloud.pubsub.api.
ce34ef3
@dhermes
Making Topic.from_api_repr accept a client.
e187345 
Also moving Topic.client -> Topic._client since it
should not (maybe?) be user facing.

Also ditching use of actual Client class in test_topic
and just using a mock (this simplifes things a bit).
@dhermes
Adding client to Subscription.from_api_repr.
d9e3dc3 
This is so that it can be passed through to the parent topic
created with the subscription.
@dhermes
Making client mandatory in most of pubsub.
8360e69 
In particular, in the Topic constructor and in
Topic.from_api_repr and Subscription.from_api_repr.
@dhermes
Using client on all Topic methods.
a1a19a3 
Ditching use of connection since client already
contains this information.
@dhermes
Using client on all Subscription methods.
d8626ef 
Ditching use of connection since client already
contains this information.
@dhermes
Removing unused implict code in pubsub.
ff39a92
@dhermes dhermes added the api: pubsub Issues related to the Pub/Sub API. label Jun 6, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 6, 2015
jgeewax
jgeewax reviewed Jun 6, 2015
View reviewed changes
docs/pubsub-usage.rst
>>> topic = Topic('topic_name')
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')

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

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 dhermes force-pushed the pubsub-client-approach branch 2 times, most recently from 838392f to 4d723d9 Compare June 6, 2015 16:55
@jgeewax jgeewax mentioned this pull request Jun 6, 2015
Closed
jgeewax
jgeewax reviewed Jun 6, 2015
View reviewed changes
docs/pubsub-usage.rst

- credentials (derived from GAE / GCE environ if present).
- :class:`Client <gcloud.pubsub.client.Client>` objects hold both a ``project``
and an authenticated connection

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@jgeewax jgeewax mentioned this pull request Jun 6, 2015
Closed
jgeewax
jgeewax reviewed Jun 6, 2015
View reviewed changes
gcloud/pubsub/client.py
"""
def __init__(self, project=None, credentials=None, http=None):
if project is None:
project = _get_production_project()

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

@jgeewax
Copy link
Contributor

jgeewax commented Jun 6, 2015

Awesome @dhermes . This LGTM but I think it may be worthwhile to have @tseaver take a look too.....

@dhermes dhermes force-pushed the pubsub-client-approach branch from 4d723d9 to 29f35d0 Compare June 6, 2015 17:12
@dhermes dhermes mentioned this pull request Jun 8, 2015
Closed
@dhermes
Copy link
Contributor Author

dhermes commented Jun 9, 2015

@tseaver You all done with your review?

@tseaver
Copy link
Contributor

tseaver commented Jun 9, 2015

Yup