Skip to content

Adding subscription factory on Pub / Sub client. - #3370

Merged
tseaver merged 2 commits into
googleapis:masterfrom
dhermes:fix-3369
May 16, 2017
Merged

Adding subscription factory on Pub / Sub client.#3370
tseaver merged 2 commits into
googleapis:masterfrom
dhermes:fix-3369

Conversation

@dhermes

@dhermesdhermes commented May 4, 2017

Copy link
Copy Markdown
Contributor

@dhermesdhermes added the api: pubsub Issues related to the Pub/Sub API. label May 4, 2017
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label May 4, 2017
@dhermesdhermes mentioned this pull request May 4, 2017
Closed
dhermes
dhermes commented May 4, 2017
View reviewed changes
Comment threadpubsub/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

dhermes commented May 9, 2017

Copy link
Copy Markdown
ContributorAuthor

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

tseaver
tseaver reviewed May 9, 2017
View reviewed changes
Comment threaddocs/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

Comment threadpubsub/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

lukesneeringer commented May 10, 2017

Copy link
Copy Markdown
Contributor

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

@kir-titievsky

kir-titievsky commented May 10, 2017 via email

Copy link
Copy Markdown

@dhermes

dhermes commented May 10, 2017

Copy link
Copy Markdown
ContributorAuthor

@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

@lukesneeringerlukesneeringer left a comment

Copy link
Copy Markdown
Contributor

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:masterMay 16, 2017
@dhermes
dhermes deleted the fix-3369 branch May 16, 2017 16:46
@tseavertseaver mentioned this pull request Jun 26, 2017
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@kir-titievskykir-titievskykir-titievsky left review comments
@lukesneeringerlukesneeringerlukesneeringer approved these changes
@tseavertseavertseaver approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lukesneeringerlukesneeringer

@tseavertseaver

Labels

api: pubsubIssues related to the Pub/Sub API.cla: yesThis 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