Skip to content

Commit

Permalink
Addressing review comments in PR #910.
Browse files Browse the repository at this point in the history
In particular:
- Renaming `with_service_...` methods to `from_service_...` on
  `pubsub.Client`.
- Implementing `topic.subscription` factory.
- Using `topic.subscription` factory in docs and regression tests
  instead of instantiating a `Subscription`.
- Updating docstring for `http` on `pubsub.Client` to describe the
  default behavior when no value is passed.
- Making `client` a required / positional argument in `pubsub.Topic`
  (and ditching a test in the process).
- Renaming `test_make_topic` as `test_topic`.
  • Loading branch information
dhermes committed Jun 9, 2015
1 parent 29f35d0 commit 1f80e36
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 33 deletions.
28 changes: 14 additions & 14 deletions docs/pubsub-usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ Authorization / Configuration

- The authentication credentials can be implicitly determined from the
environment or directly via
:meth:`with_service_account_json <gcloud.pubsub.client.Client.with_service_account_json>`
:meth:`from_service_account_json <gcloud.pubsub.client.Client.from_service_account_json>`
and
:meth:`with_service_account_p12 <gcloud.pubsub.client.Client.with_service_account_p12>`.
:meth:`from_service_account_p12 <gcloud.pubsub.client.Client.from_service_account_p12>`.

- After setting ``GOOGLE_APPLICATION_CREDENTIALS`` and ``GCLOUD_PROJECT``
environment variables, create a :class:`Client <gcloud.pubsub.client.Client>`
Expand Down Expand Up @@ -121,7 +121,7 @@ Create a new pull subscription for a topic:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> subscription.create() # API request

Create a new pull subscription for a topic with a non-default ACK deadline:
Expand All @@ -131,7 +131,7 @@ Create a new pull subscription for a topic with a non-default ACK deadline:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', ack_deadline=90)
>>> subscription = topic.subscription('subscription_name', ack_deadline=90)
>>> subscription.create() # API request

Create a new push subscription for a topic:
Expand All @@ -142,8 +142,8 @@ Create a new push subscription for a topic:
>>> ENDPOINT = 'https://example.com/hook'
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name',
... push_endpoint=ENDPOINT)
>>> subscription = topic.subscription('subscription_name',
... push_endpoint=ENDPOINT)
>>> subscription.create() # API request

Check for the existence of a subscription:
Expand All @@ -153,7 +153,7 @@ Check for the existence of a subscription:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> subscription.exists() # API request
True

Expand All @@ -165,7 +165,7 @@ Convert a pull subscription to push:
>>> ENDPOINT = 'https://example.com/hook'
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> subscription.modify_push_configuration(push_endpoint=ENDPOINT) # API request

Convert a push subscription to pull:
Expand All @@ -176,8 +176,8 @@ Convert a push subscription to pull:
>>> ENDPOINT = 'https://example.com/hook'
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubusb.Subscription('subscription_name', topic,
... push_endpoint=ENDPOINT)
>>> subscription = topic.subscription('subscription_name',
... push_endpoint=ENDPOINT)
>>> subscription.modify_push_configuration(push_endpoint=None) # API request

List subscriptions for a topic:
Expand Down Expand Up @@ -208,7 +208,7 @@ Delete a subscription:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> subscription.delete() # API request


Expand All @@ -222,7 +222,7 @@ Fetch pending messages for a pull subscription:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> with topic.batch() as batch:
... batch.publish('this is the first message_payload')
... batch.publish('this is the second message_payload',
Expand Down Expand Up @@ -251,7 +251,7 @@ Fetch a limited number of pending messages for a pull subscription:
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> with topic.batch() as batch:
... batch.publish('this is the first message_payload')
... batch.publish('this is the second message_payload',
Expand All @@ -267,7 +267,7 @@ Fetch messages for a pull subscription without blocking (none pending):
>>> from gcloud import pubsub
>>> client = pubsub.Client()
>>> topic = client.topic('topic_name')
>>> subscription = pubsub.Subscription('subscription_name', topic)
>>> subscription = topic.subscription('subscription_name')
>>> received = subscription.pull(max_messages=1) # API request
>>> messages = [recv[1] for recv in received]
>>> [message.id for message in messages]
Expand Down
8 changes: 5 additions & 3 deletions gcloud/pubsub/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ class Client(object):
from the environment.
:type http: :class:`httplib2.Http` or class that defines ``request()``.
:param http: An optional HTTP object to make requests.
:param http: An optional HTTP object to make requests. If not passed, an
``http`` object is created that is bound to the
``credentials`` for the current object.
:raises: :class:`ValueError` if the project is neither passed in nor
set in the environment.
Expand All @@ -58,7 +60,7 @@ def __init__(self, project=None, credentials=None, http=None):
self.connection = Connection(credentials=credentials, http=http)

@classmethod
def with_service_account_json(cls, json_credentials_path, project=None):
def from_service_account_json(cls, json_credentials_path, project=None):
"""Factory to retrieve JSON credentials while creating client.
:type json_credentials_path: string
Expand All @@ -81,7 +83,7 @@ def with_service_account_json(cls, json_credentials_path, project=None):
return cls(project=project, credentials=credentials)

@classmethod
def with_service_account_p12(cls, client_email, private_key_path,
def from_service_account_p12(cls, client_email, private_key_path,
project=None):
"""Factory to retrieve P12 credentials while creating client.
Expand Down
4 changes: 3 additions & 1 deletion gcloud/pubsub/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from gcloud.exceptions import NotFound
from gcloud.pubsub.message import Message
from gcloud.pubsub.topic import Topic


class Subscription(object):
Expand Down Expand Up @@ -63,6 +62,9 @@ def from_api_repr(cls, resource, client, topics=None):
:rtype: :class:`gcloud.pubsub.subscription.Subscription`
:returns: Subscription parsed from ``resource``.
"""
# pylint: disable=cyclic-import
from gcloud.pubsub.topic import Topic
# pylint: enable=cyclic-import
if topics is None:
topics = {}
t_name = resource['topic']
Expand Down
10 changes: 5 additions & 5 deletions gcloud/pubsub/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_ctor_explicit(self):
self.assertTrue(client_obj.connection._credentials is CREDS)
self.assertEqual(CREDS._scopes, SCOPE)

def test_with_service_account_json(self):
def test_from_service_account_json(self):
from gcloud._testing import _Monkey
from gcloud.pubsub import client
from gcloud.pubsub.connection import Connection
Expand All @@ -98,15 +98,15 @@ def mock_creds(arg1):

BOGUS_ARG = object()
with _Monkey(client, get_for_service_account_json=mock_creds):
client_obj = KLASS.with_service_account_json(
client_obj = KLASS.from_service_account_json(
BOGUS_ARG, project=PROJECT)

self.assertEqual(client_obj.project, PROJECT)
self.assertTrue(isinstance(client_obj.connection, Connection))
self.assertTrue(client_obj.connection._credentials is CREDS)
self.assertEqual(_CALLED, [(BOGUS_ARG,)])

def test_with_service_account_p12(self):
def test_from_service_account_p12(self):
from gcloud._testing import _Monkey
from gcloud.pubsub import client
from gcloud.pubsub.connection import Connection
Expand All @@ -123,7 +123,7 @@ def mock_creds(arg1, arg2):
BOGUS_ARG1 = object()
BOGUS_ARG2 = object()
with _Monkey(client, get_for_service_account_p12=mock_creds):
client_obj = KLASS.with_service_account_p12(
client_obj = KLASS.from_service_account_p12(
BOGUS_ARG1, BOGUS_ARG2, project=PROJECT)

self.assertEqual(client_obj.project, PROJECT)
Expand Down Expand Up @@ -302,7 +302,7 @@ def test_list_subscriptions_with_topic_name(self):
% (PROJECT, TOPIC_NAME))
self.assertEqual(req['query_params'], {})

def test_make_topic(self):
def test_topic(self):
PROJECT = 'PROJECT'
TOPIC_NAME = 'TOPIC_NAME'
CREDS = _Credentials()
Expand Down
17 changes: 14 additions & 3 deletions gcloud/pubsub/test_topic.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ def _getTargetClass(self):
def _makeOne(self, *args, **kw):
return self._getTargetClass()(*args, **kw)

def test_ctor_wo_client(self):
self.assertRaises(ValueError, self._makeOne, 'TOPIC_NAME', client=None)

def test_ctor_w_explicit_timestamp(self):
TOPIC_NAME = 'topic_name'
PROJECT = 'PROJECT'
Expand Down Expand Up @@ -334,6 +331,20 @@ def test_delete_w_alternate_client(self):
self.assertEqual(req['method'], 'DELETE')
self.assertEqual(req['path'], '/%s' % PATH)

def test_subscription(self):
from gcloud.pubsub.subscription import Subscription
TOPIC_NAME = 'topic_name'
PROJECT = 'PROJECT'
CLIENT = _Client(project=PROJECT)
topic = self._makeOne(TOPIC_NAME,
client=CLIENT)

SUBSCRIPTION_NAME = 'subscription_name'
subscription = topic.subscription(SUBSCRIPTION_NAME)
self.assertTrue(isinstance(subscription, Subscription))
self.assertEqual(subscription.name, SUBSCRIPTION_NAME)
self.assertTrue(subscription.topic is topic)


class TestBatch(unittest2.TestCase):

Expand Down
25 changes: 22 additions & 3 deletions gcloud/pubsub/topic.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,32 @@ class Topic(object):
to the attributes of each published message:
the value will be an RFC 3339 timestamp.
"""
def __init__(self, name, client=None, timestamp_messages=False):
def __init__(self, name, client, timestamp_messages=False):
self.name = name
if client is None:
raise ValueError('Topic constructor requires a client.')
self._client = client
self.timestamp_messages = timestamp_messages

def subscription(self, name, ack_deadline=None, push_endpoint=None):
"""Creates a subscription bound to the current topic.
:type name: string
:param name: the name of the subscription
:type ack_deadline: int
:param ack_deadline: the deadline (in seconds) by which messages pulled
from the back-end must be acknowledged.
:type push_endpoint: string
:param push_endpoint: URL to which messages will be pushed by the
back-end. If not set, the application must pull
messages.
"""
# pylint: disable=cyclic-import
from gcloud.pubsub.subscription import Subscription
# pylint: enable=cyclic-import
return Subscription(name, self, ack_deadline=ack_deadline,
push_endpoint=push_endpoint)

@classmethod
def from_api_repr(cls, resource, client):
"""Factory: construct a topic given its API representation
Expand Down
7 changes: 3 additions & 4 deletions regression/pubsub.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

from gcloud import _helpers
from gcloud import pubsub
from gcloud.pubsub.subscription import Subscription


_helpers._PROJECT_ENV_VAR_NAME = 'GCLOUD_TESTS_PROJECT_ID'
Expand Down Expand Up @@ -68,7 +67,7 @@ def test_create_subscription(self):
topic.create()
self.to_delete.append(topic)
SUBSCRIPTION_NAME = 'subscribing-now'
subscription = Subscription(SUBSCRIPTION_NAME, topic)
subscription = topic.subscription(SUBSCRIPTION_NAME)
self.assertFalse(subscription.exists())
subscription.create()
self.to_delete.append(subscription)
Expand All @@ -88,7 +87,7 @@ def test_list_subscriptions(self):
'newest%d' % (1000 * time.time(),),
]
for subscription_name in subscriptions_to_create:
subscription = Subscription(subscription_name, topic)
subscription = topic.subscription(subscription_name)
subscription.create()
self.to_delete.append(subscription)

Expand All @@ -106,7 +105,7 @@ def test_message_pull_mode_e2e(self):
topic.create()
self.to_delete.append(topic)
SUBSCRIPTION_NAME = 'subscribing-now'
subscription = Subscription(SUBSCRIPTION_NAME, topic)
subscription = topic.subscription(SUBSCRIPTION_NAME)
self.assertFalse(subscription.exists())
subscription.create()
self.to_delete.append(subscription)
Expand Down

0 comments on commit 1f80e36

Please sign in to comment.