Let user register callback and get acknowledgement result #18702
Conversation
NivedhaSenthil
added
the
do not merge
|
This PR is in draft stage, to be reviewed once #18395 is reviewed and merged. |
| @@ -179,9 +181,9 @@ def published_at | |||
| # # Shut down the subscriber when ready to stop receiving messages. | |||
| # subscriber.stop! | |||
| # | |||
| def acknowledge! | |||
| def acknowledge! &block | |||
There was a problem hiding this comment.
This is my lack of ruby skills: Is this change impacting existing usage of acknowledge? Same question for the rest of the changes in this class.
I see we have changes in the usage comments. Do we need to provide example for previous usage?
There was a problem hiding this comment.
This is not a usage change. The & "argument" is just a syntax for capturing any passed block as an argument.
| @@ -255,11 +255,7 @@ def streaming_pull request_enum | |||
| ## | |||
| # Acknowledges receipt of a message. | |||
| def acknowledge subscription, *ack_ids | |||
| begin | |||
There was a problem hiding this comment.
Why are we removing the try catch as part of this?
There was a problem hiding this comment.
This try catch was added when we had to ignore all the GRPC errors earlier when we were yet to add support for exactly once delivery. As we will be now propagating the error and take action based on it like retry or constructing AcknowledgementResult we no longer need to catch it and ignore here.
google-cloud-pubsub/lib/google/cloud/pubsub/received_message.rb
Outdated
Show resolved
Hide resolved
| @@ -179,9 +181,9 @@ def published_at | |||
| # # Shut down the subscriber when ready to stop receiving messages. | |||
| # subscriber.stop! | |||
| # | |||
| def acknowledge! | |||
| def acknowledge! &block | |||
There was a problem hiding this comment.
This is not a usage change. The & "argument" is just a syntax for capturing any passed block as an argument.
google-cloud-pubsub/lib/google/cloud/pubsub/acknowledge_result.rb
Outdated
Show resolved
Hide resolved
NivedhaSenthil
removed
the
do not merge
| @ack_callback_register = {} | ||
| @modack_callback_register = {} | ||
|
|
||
| @retry_thread_pool = Concurrent::ThreadPoolExecutor.new max_threads: @subscriber.callback_threads |
There was a problem hiding this comment.
Shouldn't this be @subscriber.push_threads?
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
bundle exec rake ciin the gem subdirectory.closes: #18236
closes: #18237