-
Notifications
You must be signed in to change notification settings - Fork 373
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
feat(pubsub): blocking pulls #10317
feat(pubsub): blocking pulls #10317
Conversation
This adds blocking pulls to the Pub/Sub public APIs. It also includes a minimal example, and an integration test.
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov ReportBase: 93.84% // Head: 93.84% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #10317 +/- ##
=======================================
Coverage 93.84% 93.84%
=======================================
Files 1594 1594
Lines 144970 145009 +39
=======================================
+ Hits 136044 136084 +40
+ Misses 8926 8925 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
google/cloud/pubsub/pull_response.h
Outdated
* the invocation succeeds. With best-efforts subscriptions, the service *may* | ||
* redeliver the message, even after a successful `handler.ack()` invocation. | ||
* | ||
* If `handler` is not an rvalue, you may need to use `std::move(ack).ack()` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/std::move(ack)/std::move(handler)/ ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
google/cloud/pubsub/pull_response.h
Outdated
@@ -0,0 +1,55 @@ | |||
// Copyright 2020 Google LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
} | ||
EXPECT_THAT(ids, Not(IsEmpty())); | ||
|
||
auto const count = 2 * ids.size(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we do 2*N iterations for an exactly-once subscription?
Is it to make sure everything gets acked, in the failure case where we continue
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. For low values of "make sure".
google/cloud/pubsub/subscriber.h
Outdated
* @par Idempotency | ||
* @parblock | ||
* This is an idempotent operation; it only reads messages from the service. | ||
* Will make multiple attempts to pull a message from the service, subject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Will/It will/ ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
This adds blocking pulls to the Pub/Sub public APIs. It also includes a minimal example, and an integration test.
Fixes #7187
This change is