Add endpointProvider in ClientConfig and RequestOverrideConfig. Also add crossRegionBucketAccess in S3Configuration by joviegas · Pull Request #3978 · aws/aws-sdk-java-v2

joviegas · 2023-05-08T06:24:31Z

Add endpointProvider in ClientConfig and RequestOverrideConfig. Also add crossRegionBucketAccess in S3Configuration

Note : This PR just adds the interfaces in project branch to read/writ EndpointProviders/S3 config. The logic of overriding and resolving in COntectBuilders will be addded next PR

Motivation and Context

CrossRegionBucketAccess needs the endpointProviders of the client.
Its needed to decorate endpointProviders so that region name is changed based on HeadBucket request

Modifications

Testing

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

I have read the CONTRIBUTING document
Local run of mvn install succeeds
My code follows the code style of this project
My change requires a change to the Javadoc documentation
I have updated the Javadoc documentation accordingly
I have added tests to cover my changes
All new and existing tests passed
I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

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

…add crossRegionBucketAccess in S3Configuration

...ftware/amazon/awssdk/services/serviceclientconfiguration/ServiceClientConfigurationTest.java

core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsServiceClientConfiguration.java

core/sdk-core/src/main/java/software/amazon/awssdk/core/RequestOverrideConfiguration.java

zoewangg · 2023-05-08T21:12:04Z

services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java

+
+        /**
+         * Option to enable using cross region bucket Access.
+         * With this option client can access bucket of any region.


Can we add more descriptions?

v1 javadocs may be a good starting point https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/AmazonS3Builder.java#L352

core/sdk-core/src/main/java/software/amazon/awssdk/core/RequestOverrideConfiguration.java

zoewangg · 2023-05-08T21:53:28Z

core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkServiceClientConfiguration.java

        return Optional.ofNullable(this.endpointOverride);
    }

+    public Optional<EndpointProvider> endpointProvider() {


Can we add Javadocs?

zoewangg · 2023-05-12T18:37:21Z

...st/resources/software/amazon/awssdk/codegen/poet/builder/test-sync-client-builder-class.java

+        if (clientConfiguration.option(SdkClientOption.ENDPOINT_PROVIDER) != null) {
+            endpointProvider = clientConfiguration.option(SdkClientOption.ENDPOINT_PROVIDER);
+        }


clientConfiguration.option(SdkClientOption.ENDPOINT_PROVIDER) != null seems unnecessary.

zoewangg · 2023-05-12T18:40:01Z

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-parameters.java

        return operationContextParam;
    }

+    public Builder toBuilder() {


Should this class implement ToCopyableBuilder if we are adding toBuilder method?

zoewangg · 2023-05-12T18:41:29Z

core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkServiceClientConfiguration.java


+    /**
+     *
+     * @return The configured endpoint provider of the SdkClient. If the endpoint provider was not configured, an empty


If it is not configured by users, wouldn't we return the default one created by the SDK?

zoewangg · 2023-05-12T18:47:37Z

services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java

+
+        /**
+         * <p> Configures whether cross-region bucket access is enabled for clients using the configuration.
+         * <p>The following behavior is <i>currently</i> used when this mode is enabled:


Nit: why do we need to emphasize <i>currently</i>?

zoewangg · 2023-05-12T18:49:12Z

services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java

+         *     <li>The first time a request is made that references an existing bucket (e.g., putObject API), a request will be
+         *     made to the region configured on that client to determine the region in which the bucket was created.</li>


The first time a request is made that references an existing bucket (e.g., putObject API), a request will be

Is this still true? I thought we decided to send the request as-is and redirect if needed.

Expanded this comment more to to determine the region in which the bucket was created

sonarqubecloud · 2023-05-13T03:05:22Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
15 Code Smells

74.7% Coverage
0.3% Duplication

* 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>

joviegas requested a review from a team as a code owner May 8, 2023 06:24

Add endpointProvider in ClientConfig and RequestOverrideConfig. Also …

6350e0e

…add crossRegionBucketAccess in S3Configuration

joviegas force-pushed the joviegas/cross_region_bucket_access/addConfigs branch from df32c6e to 6350e0e Compare May 8, 2023 16:29

davidh44 reviewed May 8, 2023

View reviewed changes

zoewangg reviewed May 8, 2023

View reviewed changes

updated review comments 1

d267d98

joviegas force-pushed the joviegas/cross_region_bucket_access/addConfigs branch from f71fe41 to d267d98 Compare May 12, 2023 07:59

davidh44 approved these changes May 12, 2023

View reviewed changes

zoewangg reviewed May 12, 2023

View reviewed changes

joviegas force-pushed the joviegas/cross_region_bucket_access/addConfigs branch from 6e6349e to 4f06741 Compare May 12, 2023 21:49

updated review comments 2

ae0be73

joviegas force-pushed the joviegas/cross_region_bucket_access/addConfigs branch from 4f06741 to ae0be73 Compare May 13, 2023 01:09

joviegas merged commit 496c37b into cross_region_bucket_access/project_branch May 13, 2023

joviegas deleted the joviegas/cross_region_bucket_access/addConfigs branch January 15, 2025 17:37

		* <li>The first time a request is made that references an existing bucket (e.g., putObject API), a request will be
		* made to the region configured on that client to determine the region in which the bucket was created.</li>

Conversation

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

Uh oh!

Motivation and Context

Modifications

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoewangg May 12, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented May 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joviegas commented May 8, 2023 •

edited

Loading

zoewangg May 12, 2023 •

edited

Loading