From be570db4008dc4b4dad973ef66556b4720a3cbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Tue, 4 Jun 2024 11:33:34 -0700 Subject: [PATCH 1/5] Externally named retry modes only support 'adaptive' Behind the scenes this will be mapped to RetryMode.ADAPTIVE_V2 which makes it a non-backwards compatible behavioral change. --- .../amazon/awssdk/core/retry/RetryMode.java | 2 - .../services/retry/BaseRetrySetupTest.java | 39 ++++++++++++++++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/RetryMode.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/RetryMode.java index ef84892d21d8..fdfe4fa68a82 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/RetryMode.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/RetryMode.java @@ -197,8 +197,6 @@ private static Optional fromString(String string) { case "standard": return Optional.of(STANDARD); case "adaptive": - return Optional.of(ADAPTIVE); - case "adaptive_v2": return Optional.of(ADAPTIVE_V2); default: throw new IllegalStateException("Unsupported retry policy mode configured: " + string); diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java index bb6897a74c36..3b15ab6718de 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java @@ -94,7 +94,7 @@ private void setupRetryPolicy(BuilderT builder, RetryScenario scenario) { RetryModeSetup setup = scenario.setup(); switch (setup) { case PROFILE_USING_MODE: - setupProfile(builder, scenario.mode()); + setupProfile(builder, scenario); break; case CLIENT_OVERRIDE_USING_MODE: builder.overrideConfiguration(o -> o.retryPolicy(mode)); @@ -115,7 +115,7 @@ private void setupRetryStrategy(BuilderT builder, RetryScenario scenario) { // client. switch (scenario.setup()) { case PROFILE_USING_MODE: - setupProfile(builder, scenario.mode()); + setupProfile(builder, scenario); break; case CLIENT_OVERRIDE_USING_MODE: builder.overrideConfiguration(o -> o.retryStrategy(mode)); @@ -130,8 +130,8 @@ private void setupRetryStrategy(BuilderT builder, RetryScenario scenario) { } } - private void setupProfile(BuilderT builder, RetryMode mode) { - String modeName = mode.toString().toLowerCase(Locale.ROOT); + private void setupProfile(BuilderT builder, RetryScenario scenario) { + String modeName = scenario.modeExternalName(); ProfileFile profileFile = ProfileFile.builder() .content(new StringInputStream("[profile retry_test]\n" + "retry_mode = " + modeName)) @@ -165,7 +165,7 @@ private BuilderT clientBuilder() { private void setupScenarioBefore(RetryScenario scenario) { if (scenario.setup() == RetryModeSetup.SYSTEM_PROPERTY_USING_MODE) { - System.setProperty("aws.retryMode", scenario.mode().name().toLowerCase(Locale.ROOT)); + System.setProperty("aws.retryMode", scenario.modeExternalName()); } } @@ -236,6 +236,16 @@ private static boolean isSupportedScenario(RetryScenario scenario) { return false; } + // System property or profile do not support the internal "adaptive_v2" name, only adaptive, + // and it's mapped to adaptive_v2. We mark here adaptive using profile or system property + // and map in te tests "adaptive_v2" to "adaptive" such that everything comes together at + // the end. + if (scenario.mode() == RetryMode.ADAPTIVE + && (scenario.setup() == RetryModeSetup.PROFILE_USING_MODE + || scenario.setup() == RetryModeSetup.SYSTEM_PROPERTY_USING_MODE)) { + return false; + } + // Retry policies only support the legacy ADAPTIVE mode. if (scenario.retryImplementation() == RetryImplementation.POLICY && scenario.mode() == RetryMode.ADAPTIVE_V2) { @@ -323,6 +333,25 @@ public Builder toBuilder() { return new Builder(this); } + /** + * Returns the name used externally of the given mode. This name is used in the profile `retry_mode` setting or in the + * system property. Externally, "adaptive" gets mapped to RetryMode.ADAPTIVE_V2, and "adaptive_v2" an internal name + * only and not supported externally. + */ + public String modeExternalName() { + switch (mode) { + case ADAPTIVE: + case ADAPTIVE_V2: + return "adaptive"; + case LEGACY: + return "legacy"; + case STANDARD: + return "standard"; + default: + throw new RuntimeException("Unsupported mode: " + mode); + } + } + @Override public String toString() { return mode + " " + retryImplementation + " " + setup; From 48e61cc8790b184eaa04b4882c6d2e361d1d1c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Tue, 4 Jun 2024 14:17:02 -0700 Subject: [PATCH 2/5] Sneak in a fix from the previous PR --- .../software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java | 2 +- .../software/amazon/awssdk/retries/DefaultRetryStrategy.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java index 510c77ae030b..bef57e64f301 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java @@ -79,7 +79,7 @@ public static RetryStrategy addRetryConditions(RetryStrategy strategy) { } /** - * Returns a retry strategy that do not retry. + * Returns a retry strategy that does not retry. * * @return A retry strategy that do not retry. */ diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/DefaultRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/DefaultRetryStrategy.java index 5574e0e22b5d..b09892375119 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/DefaultRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/DefaultRetryStrategy.java @@ -30,7 +30,7 @@ private DefaultRetryStrategy() { } /** - * Returns a retry strategy that do not retry. + * Returns a retry strategy that does not retry. */ public static StandardRetryStrategy doNotRetry() { return standardStrategyBuilder() From fb34e35d8dba1774ac9815d0caa29055a1dd69f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Tue, 4 Jun 2024 15:08:32 -0700 Subject: [PATCH 3/5] Fix a test that expects adaptive to map to `RetryMode.ADAPTIVE` --- .../java/software/amazon/awssdk/core/retry/RetryModeTest.java | 4 ++-- .../awssdk/services/dynamodb/DynamoDbRetryPolicyTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/RetryModeTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/RetryModeTest.java index 5ec5288fd3fa..b5fb23c37ed4 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/RetryModeTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/RetryModeTest.java @@ -50,10 +50,10 @@ public static Collection data() { // Test resolution new TestData("legacy", null, null, null, RetryMode.LEGACY), new TestData("standard", null, null, null, RetryMode.STANDARD), - new TestData("adaptive", null, null, null, RetryMode.ADAPTIVE), + new TestData("adaptive", null, null, null, RetryMode.ADAPTIVE_V2), new TestData("lEgAcY", null, null, null, RetryMode.LEGACY), new TestData("sTanDaRd", null, null, null, RetryMode.STANDARD), - new TestData("aDaPtIvE", null, null, null, RetryMode.ADAPTIVE), + new TestData("aDaPtIvE", null, null, null, RetryMode.ADAPTIVE_V2), // Test precedence new TestData("standard", "legacy", "PropertySetToLegacy", RetryMode.LEGACY, RetryMode.STANDARD), diff --git a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/DynamoDbRetryPolicyTest.java b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/DynamoDbRetryPolicyTest.java index 8ad3240f9c9e..bcb25c2504d6 100644 --- a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/DynamoDbRetryPolicyTest.java +++ b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/DynamoDbRetryPolicyTest.java @@ -88,7 +88,7 @@ void resolve_retryModeSetWithSupplier_resolvesFromSupplier() { RetryStrategy retryStrategy = DynamoDbRetryPolicy.resolveRetryStrategy(sdkClientConfiguration); RetryMode retryMode = SdkDefaultRetryStrategy.retryMode(retryStrategy); - assertThat(retryMode).isEqualTo(RetryMode.ADAPTIVE); + assertThat(retryMode).isEqualTo(RetryMode.ADAPTIVE_V2); } @Test From 3751fed0e4757fc99e6696cd1d157f063cc24d92 Mon Sep 17 00:00:00 2001 From: Manuel Sugawara Date: Wed, 5 Jun 2024 10:08:00 -0700 Subject: [PATCH 4/5] Fix typo --- .../amazon/awssdk/services/retry/BaseRetrySetupTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java index 3b15ab6718de..096f2787f08e 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java @@ -238,7 +238,7 @@ private static boolean isSupportedScenario(RetryScenario scenario) { // System property or profile do not support the internal "adaptive_v2" name, only adaptive, // and it's mapped to adaptive_v2. We mark here adaptive using profile or system property - // and map in te tests "adaptive_v2" to "adaptive" such that everything comes together at + // and map in the tests "adaptive_v2" to "adaptive" such that everything comes together at // the end. if (scenario.mode() == RetryMode.ADAPTIVE && (scenario.setup() == RetryModeSetup.PROFILE_USING_MODE From cb8b5cd261b00ad73e3e13f73b2d3114cd5630e7 Mon Sep 17 00:00:00 2001 From: Manuel Sugawara Date: Wed, 5 Jun 2024 10:08:51 -0700 Subject: [PATCH 5/5] Fix typo --- .../amazon/awssdk/services/retry/BaseRetrySetupTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java index 096f2787f08e..980b95ea9c84 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/BaseRetrySetupTest.java @@ -335,7 +335,7 @@ public Builder toBuilder() { /** * Returns the name used externally of the given mode. This name is used in the profile `retry_mode` setting or in the - * system property. Externally, "adaptive" gets mapped to RetryMode.ADAPTIVE_V2, and "adaptive_v2" an internal name + * system property. Externally, "adaptive" gets mapped to RetryMode.ADAPTIVE_V2, and "adaptive_v2" is an internal name * only and not supported externally. */ public String modeExternalName() {