Skip to content

Conversation

@chengoramazon
Copy link
Contributor

@chengoramazon chengoramazon commented Oct 1, 2024

Problem

  • /dev conversation would got terminated by retry-able errors, since we disabled the default retry.

Solution

  • Re-enable the default to prevent /dev from terminated by transient/retry-able errors.

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

github-actions bot commented Oct 1, 2024

This pull request modifies code in src/ but no tests were added/updated. Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@github-actions
Copy link

github-actions bot commented Oct 1, 2024

This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions.

@chengoramazon chengoramazon reopened this Oct 1, 2024
@chengoramazon chengoramazon changed the title fix(/dev): re-enable sdk retries to avoid /dev terminated by transient errors config(dev): re-enable Q dev sdk retries Oct 1, 2024
@chengoramazon chengoramazon marked this pull request as ready for review October 2, 2024 15:16
@chengoramazon chengoramazon changed the title config(dev): re-enable Q dev sdk retries config(dev): re-enable Q dev API retry for read APIs Oct 2, 2024

// Re-enable once BE is able to handle retries.
const writeAPIRetryOptions = {
maxRetries: 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is still at zero, so what effect does this PR have?

Copy link
Contributor Author

@chengoramazon chengoramazon Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ensure Read APIs using default 3 retries. We don't handle write retries at this moment, but we would support it in the future.

@justinmk3 justinmk3 merged commit 68f6c38 into aws:master Oct 2, 2024
10 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants