-
Notifications
You must be signed in to change notification settings - Fork 375
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(common): backoff policy clones initial state #7696
Conversation
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7696 +/- ##
=======================================
Coverage 95.29% 95.29%
=======================================
Files 1254 1254
Lines 113579 113589 +10
=======================================
+ Hits 108233 108249 +16
+ Misses 5346 5340 -6
Continue to review full report at Codecov.
|
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.
Consider updating CHANGELOG.md as part of this PR, it will be harder to remember what we wanted to say when we release this.
return std::unique_ptr<RPCBackoffPolicy>(new ExponentialBackoffPolicy(*this)); | ||
return std::unique_ptr<RPCBackoffPolicy>( | ||
new ExponentialBackoffPolicy(initial_delay_, maximum_delay_)); |
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.
Possible follow-up note: It seems to me that copy construction/assignment for these "clone-able" classes is an attractive nuisance (they don't support equality after all), and that we'd be well served trying to delete that capability.
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
When calling
.clone()
on an exponential backoff policy, copy the initial state instead of the current state. This may result in a change of behavior, but we see it as a bug that ought to be fixed.This change is