-
Notifications
You must be signed in to change notification settings - Fork 29
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: Retry handling for streamed request bodies #852
Conversation
@@ -167,20 +168,22 @@ class OrchestratorTests: XCTestCase { | |||
} | |||
|
|||
class TraceExecuteRequest: ExecuteRequest { |
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.
The behavior of this type is unchanged; I added a variable to allow for counting of retry attempts that is separate from the original allowed count.
@@ -1315,4 +1318,43 @@ class OrchestratorTests: XCTestCase { | |||
} | |||
} | |||
} | |||
|
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.
The RetryBackoffStrategy
below always sets a retry delay of 0 so the retry tests can proceed without waiting for any retry delay. This prevents the first test below from taking 0.5-2 seconds to finish.
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.
LGTM
Description of changes
This change ensures that retry is only used when a request body is reusable, and that the request body is prepared for reuse if needed.
Additionally, unit tests are added to
Orchestrator
to verify correct retry behavior.Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.