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

Pubsub Could Use Autocreate on Topics and Subscriptions #905

Closed
waprin opened this issue Jun 1, 2015 · 12 comments
Closed

Pubsub Could Use Autocreate on Topics and Subscriptions #905

waprin opened this issue Jun 1, 2015 · 12 comments
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API.
Milestone

Comments

@waprin
Copy link
Contributor

waprin commented Jun 1, 2015

I think this will be a recurring pattern:

topic = pubsub.Topic('book-process-queue')
if not topic.exists():
    topic.create()

subscription = pubsub.Subscription('book-sub', topic)
if not subscription.exists():
    subscription.create()

So it might be a good idea to simplify it to:

topic = pubsub.Topic('book-process-queue', auto_create=True)

gcloud-node does something like this already.

@dhermes dhermes added the api: pubsub Issues related to the Pub/Sub API. label Jun 1, 2015
@dhermes dhermes added this to the Pub/Sub Alpha milestone Jun 1, 2015
@dhermes
Copy link
Contributor

dhermes commented Jun 3, 2015

@tmatsuo WDYT?

@tmatsuo
Copy link
Contributor

tmatsuo commented Jun 3, 2015

Yeah, that seems convenient, although there are some caveats.

  • A topic created by auto_create=True will be a dev-null topic which means that messages will be just dropped.
  • Implementation should be done in a way that the additional API call is made only when necessary (Don't always check the existence).

@dhermes
Copy link
Contributor

dhermes commented Jun 9, 2015

@tmatsuo

  • What is a dev-null topic?
  • How can one avoid checking existence?

@theacodes
Copy link
Contributor

We had this same conversation at gcloud-node here.

Basically what it boils down to is:

  • a dev-null topic is a topic without subscriptions, so any messages sent will be dropped. I think it's out of the scope of the client libraries to avoid this (as it's not an error state), as it should be clearly expressed in docs and samples.
  • You can avoid checking by lazily creating. So, create when you get a 404 and retry the original request. That's what we did in node here.

@dhermes
Copy link
Contributor

dhermes commented Jun 9, 2015

How is that lazy? That's a GET followed by a POST if unsuccessful.

@theacodes
Copy link
Contributor

The alternative would be to check at construction or before a request. In
most cases the resource does exist, so its a superfluous request.

On Tue, Jun 9, 2015, 4:43 PM Danny Hermes [email protected] wrote:

How is that lazy? That's a GET followed by a POST if unsuccessful.


Reply to this email directly or view it on GitHub
#905 (comment)
.

@jgeewax
Copy link
Contributor

jgeewax commented Jun 10, 2015

I agree, it's "lazy" in the sense that "we'll go based on the assumption that it exists" and a 404 means something different to that kind of topic. 404 to an autocreate topic means "go create the topic, then call the same thing again".

@tseaver tseaver changed the title Pubsub Could Use Autocreate no Topics and Subscriptions Pubsub Could Use Autocreate on Topics and Subscriptions Jun 10, 2015
@dhermes
Copy link
Contributor

dhermes commented Jun 10, 2015

@jonparrott Maybe I skimmed the node code too quickly / don't understand.

You're saying behavior like this

# foo does NOT exist
topic = Topic('foo', auto_create=True)  # No HTTP request is sent
message = ...
topic.publish(message)  # HTTP request results in 404, so first create and then publish

and

# foo DOES exist
topic = Topic('foo', auto_create=True)  # No HTTP request is sent
message = ...
topic.publish(message)  # HTTP request results in 200, confirmed the item exists

Here is some issues I notice with the node approach:

  • In the failure case, publish results in 3 requests (404, 200 and 200). We could just as well do a create requests (PUT /projects/{project}/topics/{topic_name}) and just take a 409 or a 200 as a good sign and raise in any other case. This would just be 2 requests to publish (409/200 and 200).
  • Tracking whether we are sure or not the item has been created should be done. In other words, a 404 for another reason should not trigger the create request (should not even do it a 2nd time).

@dhermes
Copy link
Contributor

dhermes commented Jun 10, 2015

I'm going to send a fix and we can discuss a bit there?

@theacodes
Copy link
Contributor

In the failure case, publish results in 3 requests

Correct, but the failure case only happens once.

We could just as well do a create requests.

You could, but you would need to do this every time the item is constructed.

The trade off is that assuming it exists and handling the failure cases results in 2 extra requests once, whereas looking before you leap means an extra request every single time.

In other words, a 404 for another reason should not trigger the create request

The node one only tries to create once. If the creation method fails, it returns the error to the original callback.

@dhermes
Copy link
Contributor

dhermes commented Jun 10, 2015

I definitely overlooked "whereas looking before you leap means an extra request every single time."

Thanks

dhermes added a commit to dhermes/google-cloud-python that referenced this issue Jun 25, 2015
@dhermes
Copy link
Contributor

dhermes commented Jun 30, 2015

Closing. See #949 for context.

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.
Projects
None yet
Development

No branches or pull requests

5 participants