Make timeout per-request #1181
Merged
Make timeout per-request #1181
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Kern-- Thank you for the patch, looks good to me. Could you rebase thie patch on the latest main branch for fixing the CI failure? |
When setting up an rhttp client, the structure looks like this:
```
http.Client {
Transport: rhttp.Transport {
Client: rhttp.Client {
HTTPClient: http.Client{}
}
}
}
```
Before this change, the timeout was set on the outer client. When the
timeout was reached, the request context was cancelled and no more
retries would be attempted by the retryable client. Effectively the
timeout was being set for the entire retryable request.
This change moves the timeout to the inner client, making the timeout
per-request and the timeout of the entire retryable request is governed
by the retry policy.
Signed-off-by: Kern Walster <walster@amazon.com>
ktock
approved these changes
Mar 28, 2023
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #1180
When setting up an rhttp client, the structure looks like this:
Before this change, the timeout was set on the outer client. When the timeout was reached, the request context was cancelled and no more retries would be attempted by the retryable client. Effectively the timeout was being set for the entire retryable request.
This change moves the timeout to the inner client, making the timeout per-request and the timeout of the entire retryable request is governed by the retry policy.