Skip to content
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(maxAttempts): Add new configuration maxAttempts in RemoteProvider… #1346

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/red-coats-visit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@smithy/credential-provider-imds": minor
---

The configuration maxRetries was changed to maxAttempts in middleware-retry to be compliant with other SDKs and retry strategy #1244. This change adds new configuration maxAttempts in RemoteProviderConfig and deprecates maxRetries.
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,48 @@ describe("providerConfigFromInit", () => {
maxRetries,
});
});

it("should return maxAttempts with the same value as the deprecated maxRetries if maxAttempts is NOT provided", () => {
const timeout = 123456789;
const maxRetries = 987654321;

expect(providerConfigFromInit({ timeout, maxRetries })).toEqual({
timeout,
maxRetries,
maxAttempts: maxRetries,
});
});

it("should return maxAttempts with the same value as maxAttempts option if it is provided", () => {
const timeout = 123456789;
const maxRetries = 987654321;
const maxAttempts = 987654322;

expect(providerConfigFromInit({ timeout, maxRetries, maxAttempts })).toEqual({
timeout,
maxRetries,
maxAttempts,
});
});

it("should return maxAttempts with DEFAULT_MAX_RETRIES as value if both maxAttempts and maxRetries are NOT provided", () => {
const timeout = 123456789;

expect(providerConfigFromInit({ timeout })).toEqual({
timeout,
maxRetries: 0,
maxAttempts: 0,
});
});

it("should return maxRetries with the same value as the new maxRetries maxAttempts if maxAttempts is provided with any value", () => {
const timeout = 123456789;
const maxAttempts = 987654321;

expect(providerConfigFromInit({ timeout, maxAttempts })).toEqual({
timeout,
maxRetries: maxAttempts,
maxAttempts,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ export interface RemoteProviderConfig {
timeout: number;

/**
* @deprecated The configuration maxRetries was changed to maxAttempts in middleware-retry to be compliant with other SDKs and retry strategy [#1244](https://github.com/aws/aws-sdk-js-v3/pull/1244). Use maxAttempts instead.
* The maximum number of times the HTTP connection should be retried
*/
maxRetries: number;
maxRetries?: number;

/**
* The maximum number of times the HTTP connection should be retried
*/
maxAttempts?: number;
}

/**
Expand All @@ -47,5 +53,14 @@ export interface RemoteProviderInit extends Partial<RemoteProviderConfig> {
*/
export const providerConfigFromInit = ({
maxRetries = DEFAULT_MAX_RETRIES,
maxAttempts,
timeout = DEFAULT_TIMEOUT,
}: RemoteProviderInit): RemoteProviderConfig => ({ maxRetries, timeout });
}: RemoteProviderInit): RemoteProviderConfig => {
const effectiveMaxAttempts = maxAttempts || maxRetries;

return {
maxRetries: effectiveMaxAttempts,
maxAttempts: effectiveMaxAttempts,
timeout
};
}