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

#744: Add 'pubsub.message.Message' class #800

Merged
merged 2 commits into from
Apr 6, 2015
Merged

#744: Add 'pubsub.message.Message' class #800

merged 2 commits into from
Apr 6, 2015

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Apr 6, 2015

Also, convert factory helper functions for topics and subscriptions into from_api_repr class methods.

- 'api._topic_from_resource' -> 'Topic.from_api_repr'
- 'api._subscription_from_resource' -> 'Subscription.from_api_repr'
@tseaver tseaver added the api: pubsub Issues related to the Pub/Sub API. label Apr 6, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 6, 2015
@tseaver tseaver mentioned this pull request Apr 6, 2015
9 tasks

@property
def attrs(self):
"""Lazily-constructed attribute dictionary"""

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Apr 6, 2015

What's the necessity of a Message class? In our original document, it wasn't clear if one would ever need to exist.

@tseaver
Copy link
Contributor Author

tseaver commented Apr 6, 2015

Mostly, the message has some behavior that differs from the raw mappings: base64-decoding the payload, for one. If we need to add support for timestamping / serial handling, then it gives us a place to hang it on the subscriber side.

@dhermes
Copy link
Contributor

dhermes commented Apr 6, 2015

OK LGTM. I'm not sure I buy the savings of using None instead of {} on an object, but not worth getting hung up on.

Maps 'pubsub.v1beta2.PubsubMessage', handles base64-decode of payload.

Return 'Message' instances from 'Subscription.pull()'.

Update docs accordingly.
@tseaver
Copy link
Contributor Author

tseaver commented Apr 6, 2015

Squashed down to 2 commits. I will merge when Travis pulls its thumb out.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6a5f480 on tseaver:744-pubsub-message_class into * on GoogleCloudPlatform:master*.

tseaver added a commit that referenced this pull request Apr 6, 2015
@tseaver tseaver merged commit 9c7f1fd into googleapis:master Apr 6, 2015
@tseaver tseaver deleted the 744-pubsub-message_class branch April 6, 2015 22:19
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