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

Adding subscription factory on Pub / Sub client. #3370

Merged
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
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.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented May 9, 2017

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

SUBSCRIPTION_NAME = 'subscription_factory-%d' % (_millis(),)

# [START client_subscription]
subscription = client.topic(

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

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.

@lukesneeringer
Copy link
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
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.)

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
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.

5 participants