Integration test cases for Cross region Async and Sync Clients#4128
Merged
joviegas merged 5 commits intocross_region_bucket_access/project_branchfrom Jun 23, 2023
Merged
Conversation
7f35b68 to
cfc2eb6
Compare
dagnir
reviewed
Jun 22, 2023
services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/CaptureInterceptor.java
Outdated
Show resolved
Hide resolved
| import software.amazon.awssdk.services.s3.paginators.ListObjectsV2Publisher; | ||
|
|
||
| public class S3AsyncCrossRegionIntegrationTest extends S3CrossRegionIntegrationTestBase { | ||
| private S3AsyncClient crossRegionS3Client ; |
...it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionIntegrationTestBase.java
Outdated
Show resolved
Hide resolved
| DEFAULT_REGION.id())); | ||
| assertThat(captureInterceptor.httpMethods()).isEqualTo(Arrays.asList(SdkHttpMethod.DELETE, SdkHttpMethod.DELETE)); | ||
| assertThat(response).isNotNull(); | ||
| assertThat(captureInterceptor.getServiceCalls()).isEqualTo(2); |
Contributor
There was a problem hiding this comment.
This seems like it would be flaky if there are ever any retries. Is there some way to rewrite the checks so that it's more resilient to retries?
Contributor
Author
There was a problem hiding this comment.
Good catch.
Removed the interceptors
Testing using interceptors is covered in Junits.
Now I just make sure the responses from Cross region calls are as expected
1 task
…s/cross_region_integ_tests
dagnir
reviewed
Jun 23, 2023
...it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionIntegrationTestBase.java
Outdated
Show resolved
Hide resolved
fc476a6 to
036c756
Compare
036c756 to
914f62b
Compare
dagnir
approved these changes
Jun 23, 2023
|
SonarCloud Quality Gate failed.
|
joviegas
added a commit
that referenced
this pull request
Jul 5, 2023
* 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>
L-Applin
pushed a commit
that referenced
this pull request
Jul 24, 2023
* 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>
L-Applin
pushed a commit
that referenced
this pull request
Jul 24, 2023
* 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.











Motivation and Context
Modifications
Tested for getAPI service call in S3CrossRegionIntegrationTestBase
Testing with Default and Cross region Client
Tested for getAPI service call in S3CrossRegionIntegrationTestBase
ASYNC_CLIENT
###SYNC_CLIENT