-
Notifications
You must be signed in to change notification settings - Fork 751
fix(amazonq): increase GetTransformation maxRetries #6220
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
Conversation
|
| ): Promise<PromiseResult<CodeWhispererUserClient.GetTransformationResponse, AWSError>> { | ||
| return (await this.createUserSdkClient()).getTransformation(request).promise() | ||
| // instead of the default of 3 retries, use 8 retries for this API which is polled every 5 seconds | ||
| return (await this.createUserSdkClient(8)).getTransformation(request).promise() |
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 SDK uses 3 retries by default and 100 ms as the base delay during the exponential backoff retry strategy.
This means that if GetTransformation fails after only 0.1 + 0.2 + 0.4 = 0.7 seconds of delays, the transformation is marked as failed on the IDE and users are unable to view the transformation results. Increasing the retries to 8 means that GetTransformation now has a maximum of ~25 seconds of delays.
We have had customer cases where the polling fails, but the transformation eventually succeeds in our backend. This PR should increase the likelihood that those customers are able to view the transformation results.
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.
8 is admittedly arbitrary, and perhaps there's a better way to solve this problem, but I do think that permitting more than 0.7 seconds of delays is a good idea for this specific API. Open to other suggestions.
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 was going to ask why 8, but thank you for explaining that!
justinmk3
left a comment
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.
Needs a review by someone from your team
## Problem Customers that experience transient network issues are unable to view their transformation results when the `GetTransformation` API fails after the default of 3 retries. ## Solution Increase retries to 8.
Problem
Customers that experience transient network issues are unable to view their transformation results when the
GetTransformationAPI fails after the default of 3 retries.Solution
Increase retries to 8.
feature/xbranches will not be squash-merged at release time.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.