fix: introduce S3-specific client configuration to top-level configuration #378
Conversation
| @@ -286,6 +288,11 @@ public static class Builder implements AwsAsyncClientBuilder<Builder, S3AsyncEnc | |||
| private ClientAsyncConfiguration _clientAsyncConfiguration = null; | |||
| private SdkAsyncHttpClient _sdkAsyncHttpClient = null; | |||
| private SdkAsyncHttpClient.Builder _sdkAsyncHttpClientBuilder = null; | |||
| private S3Configuration _serviceConfiguration = null; | |||
| private Boolean _accelerate = null; | |||
There was a problem hiding this comment.
Curious why Boolean and not boolean? Other fields in these classes are booleans
There was a problem hiding this comment.
The SDK uses Boolean and I prefer to match the SDK for its configuration.
There was a problem hiding this comment.
Is that preferable to maintaining type consistency within our own client?
Right now our client uses both boolean and Boolean.
I'd prefer that our own client is consistent within itself (i.e. exclusively uses one or the other),
over having some fields be consistent with what the SDK uses.
That said, I don't understand the impact of mixing these types, but I imagine it's low and this is almost certainly just a pedantic conversation. If it is, just let me know and I'll drop this thread.
There was a problem hiding this comment.
Mixing them is fine, IMO, since they have different semantics. boolean cannot be null, whereas Boolean can. I prefer using boolean when possible so as to avoid having to handle null. There really isn't any advantage to using Boolean internally, but there is here as it maintains compatibility with the SDK.
## [3.2.3](v3.2.2...v3.2.3) (2024-10-04) ### Fixes * catch CompletionException instead when instruction file is not found ([#379](#379)) ([dd61547](dd61547)) * handle S3 server encoding of non-US-ASCII object metadata ([#375](#375)) ([b907743](b907743)) * introduce S3-specific client configuration to top-level configuration ([#378](#378)) ([54fa0cb](54fa0cb)) ### Maintenance * fix release cfn and codebuild ([#380](#380)) ([2844498](2844498)) * fix release javadocs ([#373](#373)) ([8a69959](8a69959))
Issue #, if available:
Description of changes:
In #328, the ability to configure the S3EncryptionClient and S3AsyncEncryptionClient's inner clients using the "top-level" builder was added. However, this was done based off of the more generic SDK client builder interface and thus omitted some other client configuration options specific to S3. This PR adds those extra S3-specific options to the top-level S3EC / S3AsyncEC builder.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: