Should 'pubsub.subscription.Subscription.pull()' support auto-acknowledge

[tseaver]

In pull mode, the subscriber is responsible for sending an acknowledge request after fetching messages via pull. While separating the two allows for some advanced usage (e.g., to allow "peeking" at the queue, or to allow retries after certain kinds of errors), some applications might find having to manage the manual acknowledge burdensome.

Should we add a boolean acknowledge_immediately argument to Subscription.pull()? If so, what should its default value be?

[dhermes]

/cc @tmatsuo

[tmatsuo]

@tseaver
While I understand the motivation, I would rather not have acknowledge_immediately, since it will easily lead to message losses when the process dies after the ack call succeeds.

Rather, how about to introduce a syntax sugar for making ack call easily so that the users can just add one line or so to make it work (ideally with automatic retry and async option)?

[tseaver]

@tmatsuo would something like the following be what you had in mind?

with pubsub.subscription.Acknowledge(subscription.pull(max_requests=10) as ack:
    for message in ack.messages:
         do_something_with(message)

where the Acknowledge instance is a context manager, which issues the acknowledge request in __exit__ IFF no exception has been raised?

[tmatsuo]

@tseaver
Very interesting. My little concern is about partial successes/failures. How can you acknowledge partially in that case?

How about to have items property and remove method as well as messages:

with pubsub.subscription.AutoAck(subscription.pull(max_requests=10)) as auto_ack:
    for ack_id, message in auto_ack.items:
        try:
            do_something_with(message)
        except Exception as e:
            auto_ack.remove(ack_id)

not so beautiful, but it does the job, while you can still omit the try-except clause and use messages property to allow the simplest use-case.

Another little concern was about how to call modifyAckDeadline, but on the second thought, it's not limited to this context manager.

[dhermes]

@tseaver Shall we revive or kill this?

[jgeewax]

I think maybe I'm missing the point of this -- and now getting more bummed we didn't wrap these as a bundle that I can call methods on....

for message in subscription.pull(max_requests=10):
  with message.auto_ack():
    print 1/int(message.body)
    # if an exception happens, this will fail, and the message won't get ack'd
    # (ie, if body == 0)

[tseaver]

@jgeewax Your example makes an acknowledge API request per message, rather than making a single acknowlege API request for the successfully-processed messages.

[tseaver]

@jgeewax Another issue: your example aborts processing messages beyond the first one which raises an error: if you add in the code to continue, then it becomes as complicated as @tmatsuo's version:

for message in subscription.pull(max_requests=10):
    try:
        with message.auto_ack():
            print 1 / int(message.body)
            # if an exception happens, this will fail, and the message
            # won't get ack'd, (i.e., if body == 0)
    except Exception as e:
        logger.exception(e)