-
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
docs(pubsub): add optimistic subscribe example #14272
Conversation
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14272 +/- ##
=======================================
Coverage 93.26% 93.26%
=======================================
Files 2213 2213
Lines 193279 193325 +46
=======================================
+ Hits 180266 180309 +43
- Misses 13013 13016 +3 ☔ View full report in Codecov by Sentry. |
@@ -129,7 +129,8 @@ void ExampleStatusOr(google::cloud::pubsub_admin::TopicAdminClient client, | |||
// The actual type of `topic` is | |||
// google::cloud::StatusOr<google::pubsub::v1::Topic>, but | |||
// we expect it'll most often be declared with auto like this. | |||
for (auto& topic : client.ListTopics(project_id)) { | |||
for (auto& topic : |
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.
This is an unrelated change, but I noticed it when I ran the auto version of samples
OptimisticSubscribe({project_id, topic_id, subscription_id}); | ||
|
||
std::cout << "\nRunning OptimisticSubscribe() sample [3]" << std::endl; | ||
OptimisticSubscribe({project_id, topic_id, nonexistent_subscription_id}); |
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.
good idea
return gc::Status(); | ||
} | ||
if (response.status().code() == gc::StatusCode::kUnavailable && | ||
response.status().message() == "no messages returned") { |
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.
hm, I don't love that we are recommending code that parses an error message. We make no guarantees about the contents of the error message. It's ok, though.
optional, and not for this PR: This could be an opportunity for that client origin stuff. Something that feels more official?
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.
yeah I agree... I think using IsClientOrigin would be better. I can refactor in a later PR.
This change is