Skip to content

Adding subscription factory on Pub / Sub client. #3370

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 2 commits into from
May 16, 2017
Merged

Adding subscription factory on Pub / Sub client. #3370

tseaver merged 2 commits into from
May 16, 2017

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented May 4, 2017

Fixes #3369.

/cc @kir-titievsky

@dhermes dhermes added the api: pubsub Issues related to the Pub/Sub API. label May 4, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 4, 2017
@dhermes dhermes mentioned this pull request May 4, 2017
Closed
dhermes
dhermes commented May 4, 2017
View reviewed changes
pubsub/google/cloud/pubsub/client.py
return Subscription(
name, ack_deadline=ack_deadline, push_endpoint=push_endpoint,
retain_acked_messages=retain_acked_messages,
message_retention_duration=message_retention_duration, client=self)

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
Copy link
Contributor Author

dhermes commented May 9, 2017

@tseaver @lukesneeringer @jonparrott Can you weigh in on this feature?

tseaver
tseaver reviewed May 9, 2017
View reviewed changes
docs/pubsub_snippets.py
"""Subscription factory."""
SUBSCRIPTION_NAME = 'subscription_factory-%d' % (_millis(),)

# [START client_subscription]

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

pubsub/google/cloud/pubsub/client.py
return Subscription(
name, ack_deadline=ack_deadline, push_endpoint=push_endpoint,
retain_acked_messages=retain_acked_messages,
message_retention_duration=message_retention_duration, client=self)

This comment was marked as spam.

Sign in to view

@lukesneeringer
Copy link
Contributor

lukesneeringer commented May 10, 2017

As a note, we are about to do a huge rewrite on this, so this PR is going to be moot soon.

@kir-titievsky
Copy link

kir-titievsky commented May 10, 2017 via email

@dhermes
Copy link
Contributor Author

dhermes commented May 10, 2017

@kir-titievsky @lukesneeringer Let's just merge this as-is (without topic or topic_name)? (Can sort out the rewrite after the fact.)

lukesneeringer
lukesneeringer approved these changes May 16, 2017
View reviewed changes
Copy link
Contributor

@lukesneeringer lukesneeringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Merge it.

@tseaver tseaver merged commit 469d675 into googleapis:master May 16, 2017
@dhermes dhermes deleted the fix-3369 branch May 16, 2017 16:46
@tseaver tseaver mentioned this pull request Jun 26, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@kir-titievsky kir-titievsky kir-titievsky left review comments

@lukesneeringer lukesneeringer lukesneeringer approved these changes

@tseaver tseaver tseaver approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lukesneeringer lukesneeringer

@tseaver tseaver

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.

5 participants

@dhermes @lukesneeringer @kir-titievsky @tseaver @googlebot