-
Notifications
You must be signed in to change notification settings - Fork 711
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
Added RetryConditions to Request #436
Added RetryConditions to Request #436
Conversation
@jeevatkm any updates on this? |
request.go
Outdated
@@ -747,7 +758,7 @@ func (r *Request) Execute(method, url string) (*Response, error) { | |||
Retries(r.client.RetryCount), | |||
WaitTime(r.client.RetryWaitTime), | |||
MaxWaitTime(r.client.RetryMaxWaitTime), | |||
RetryConditions(r.client.RetryConditions), | |||
RetryConditions(append(r.client.RetryConditions, r.retryConditions...)), |
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.
@rohitkg98 If we are adding a Retry condition feature at the request level; is it a request retry condition that should take place first instead of the client?
@moorereason your thoughts!
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.
@rohitkg98 Basically, Request retry conditions have to execute first and then client instance level. So we need to flip the append arguments.
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.
I am looking for this feature as well. The most specific (request specific retry condition), should apply first.
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.
changed to check request specific conditions before client ones
Codecov Report
@@ Coverage Diff @@
## master #436 +/- ##
==========================================
+ Coverage 96.06% 96.60% +0.54%
==========================================
Files 10 10
Lines 1041 1325 +284
==========================================
+ Hits 1000 1280 +280
- Misses 23 27 +4
Partials 18 18
Continue to review full report at Codecov.
|
Stoked for a merge |
?? |
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.
@rohitkg98 Thanks for the PR and contribution.
When will this be released? I updated my go.mod dependency to 2.7.0 and it isn't resolving |
Closes #433
Closes #315
Closes #324