Cross region support for CRT Client by joviegas · Pull Request #4129 · aws/aws-sdk-java-v2

joviegas · 2023-06-22T14:23:20Z

Motivation and Context

Cross region support for CRT Client

Modifications

Api for crossRegionEnabled in Crt client
Pass AwsSigning Config to crt if SigningRegion is changed

Testing

Integ test for get/put/copy.

License

I confirm that this pull request can be released under the Apache 2 license

davidh44 · 2023-06-22T18:50:00Z

...c/test/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClientTest.java

+    }
+
+    @Test
+    void crtCrtClient_with_crossRegionAccessEnabled_asFalse(){


nit: just one "crt" here and above?

davidh44 · 2023-06-22T18:50:19Z

services/s3/src/it/java/software/amazon/awssdk/services/s3/crt/TestResponseTransformer.java

+import software.amazon.awssdk.http.async.SimpleSubscriber;
+import software.amazon.awssdk.services.s3.model.GetObjectResponse;
+
+public final class TestResponseTransformer implements AsyncResponseTransformer<GetObjectResponse, Void> {


what is the use case for this class?

Will remove this from S3CrtGetObjectIntegrationTest

dagnir · 2023-06-22T22:03:34Z

...s/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java

            .withResumeToken(resumeToken);

+        // Create a new SigningConfig object only if the signing region has changed from the previously configured region.
+        if (signingRegion != null && !s3ClientOptions.getRegion().equals(signingRegion)) {


s3ClientOptions.getRegion().equals(signingRegion) is comparing different types, String and Region. Should be using signingRegion.id()

Good catch !

...s/s3/src/it/java/software/amazon/awssdk/services/s3/crt/S3CrossRegionCrtIntegrationTest.java

sonarqubecloud · 2023-06-23T15:25:46Z

SonarCloud Quality Gate failed.

5 Bugs
0 Vulnerabilities
0 Security Hotspots
77 Code Smells

89.5% Coverage
0.1% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

* Adding a generic method invokeOperation for abstract delegating clien… (#3996) Adding a generic method invokeOperation for abstract delegating client to allow overriding behavior changes for all operations * Add endpointProvider in ClientConfig and RequestOverrideConfig. Also add crossRegionBucketAccess in S3Configuration (#3978) * Add endpointProvider in ClientConfig and RequestOverrideConfig. Also add crossRegionBucketAccess in S3Configuration * updated review comments 1 * updated review comments 2 * Adds S3CrossRegion clients and codegen for retrieving bucket name (#4008) * Adds S3CrossRegion clients and codegen for retrieving bucket name * Removes codegen of bucket parameter since getValueForField() is better * Add back overloads * Adds cross region client logic for decorating endpoint provider (#4026) * Adds cross region client logic for decorating endpoint provider * Paginated methods returning publisher or iterable implement logic in … (#4046) * Paginated methods returning publisher or iterable implement logic in interface instead of throwing unsupported exception * Added internal annotation to class * Wraps s3 client in cross regional client when enabled (#4080) * Wraps s3 client in cross regional client when enabled * Adds composer interface * Add User Agent Api name for Cross region API calls (#4105) * Add User Agent Api name for Cross region API calls * Added test case for default client not to have user agent related to cross region * S3CrossRegion Sync and Async Clients Redirect implementation (#4089) * S3CrossRegionSyncClient Redirect implementation * Added implementation for Async client Decorator * Updated older Cross region test cases * Added paramterized test * Async Exception checged to completableException * Updated test cases and changes the Exception handling when HeadBucket Call fails * Handled Anna-karin's comments * Removed async execution of HeadBucket and attached it to the completableFuture of main request * Handled Zoe's comments * Added test case when Redirected after the Region is cached * Changed region constant to Region Type in Tests * Cross region support for CRT Client (#4129) * Cross region support for CRT Client * removing common class * handled review comments * Integration test cases for Cross region Async and Sync Clients (#4128) * Integration test cases for Cross region Sync and Sync Clients * Renamed files as Integration test * Handled CR comments to take care of retries thus captureInterceptor is removed * rebased and added CRT in Integ test * removed sout from integ * Attach crossRegionAccessEnabled as CustomClientParams to a client Builder, also mention of Composers are renamed as decorators (#4151) * Attach crossRegionAccessEnabled as CustomClientParams to a client Builder, also mention of Composers are renamed as decorators * Removed from S3Configurations * Move the optional parameter in getter --------- Co-authored-by: Anna-Karin Salander <salande@amazon.com>

…ffa4f5874 Pull request: release <- staging/4e1b367d-6bf1-4047-b3c0-5bdffa4f5874

joviegas requested a review from a team as a code owner June 22, 2023 14:23

Cross region support for CRT Client

2140bcd

joviegas force-pushed the joviegas/cross_region/crt_client_support branch from 2c93fcf to 2140bcd Compare June 22, 2023 14:26

davidh44 reviewed Jun 22, 2023

View reviewed changes

removing common class

535980b

joviegas force-pushed the joviegas/cross_region/crt_client_support branch from 7dfb8c4 to 535980b Compare June 22, 2023 21:31

davidh44 approved these changes Jun 22, 2023

View reviewed changes

dagnir reviewed Jun 22, 2023

View reviewed changes

handled review comments

b1178cb

dagnir approved these changes Jun 23, 2023

View reviewed changes

joviegas merged commit 4039f39 into cross_region_bucket_access/project_branch Jun 23, 2023

joviegas deleted the joviegas/cross_region/crt_client_support branch August 30, 2023 22:58

aws-sdk-java-automation added a commit that referenced this pull request Jul 16, 2025

Merge pull request #4129 from aws/staging/4e1b367d-6bf1-4047-b3c0-5bd…

7d51b43

…ffa4f5874 Pull request: release <- staging/4e1b367d-6bf1-4047-b3c0-5bdffa4f5874

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross region support for CRT Client#4129

Cross region support for CRT Client#4129
joviegas merged 3 commits intocross_region_bucket_access/project_branchfrom
joviegas/cross_region/crt_client_support

joviegas commented Jun 22, 2023

Uh oh!

davidh44 Jun 22, 2023

Uh oh!

joviegas Jun 22, 2023

Uh oh!

davidh44 Jun 22, 2023

Uh oh!

joviegas Jun 22, 2023 •

edited

Loading

Uh oh!

dagnir Jun 22, 2023

Uh oh!

joviegas Jun 23, 2023

Uh oh!

Uh oh!

sonarqubecloud bot commented Jun 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

joviegas commented Jun 22, 2023

Motivation and Context

Modifications

Testing

License

Uh oh!

davidh44 Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

joviegas Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

davidh44 Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

joviegas Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dagnir Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

joviegas Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Jun 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joviegas Jun 22, 2023 •

edited

Loading