fix: add forcePathStyle and enableAccelerate config properties back to the S3 client #1099
Conversation
| @@ -37,10 +54,9 @@ class ClientConfigIntegration : KotlinIntegration { | |||
| """.trimIndent() | |||
| } | |||
|
|
|||
| // FIXME: default signer doesn't yet implement sigv4a, default to mrap OFF until it does | |||
There was a problem hiding this comment.
We select auth scheme based on the model not on endpoint rules (yet) and we don't support sigv4a yet no matter what. Whether it's enabled or not isn't going to matter I don't think so I just changed it to the "correct" default.
There was a problem hiding this comment.
This still causes changes to how our endpoints are resolved. I think we should leave this disabled until we support MRAP in the default case.
There was a problem hiding this comment.
not a hill worth dying on, reverting.
| } | ||
|
|
||
| @Test | ||
| fun testConfigPropertiesPresent() { |
There was a problem hiding this comment.
That these properties don't accidentally disappear again from codegen.
There was a problem hiding this comment.
That's not immediately clear from the code. Can we leave a comment indicating why we have a test with no assertions?
|
A new generated diff is ready to view. |
| @@ -37,10 +54,9 @@ class ClientConfigIntegration : KotlinIntegration { | |||
| """.trimIndent() | |||
| } | |||
|
|
|||
| // FIXME: default signer doesn't yet implement sigv4a, default to mrap OFF until it does | |||
There was a problem hiding this comment.
This still causes changes to how our endpoints are resolved. I think we should leave this disabled until we support MRAP in the default case.
| } | ||
|
|
||
| @Test | ||
| fun testConfigPropertiesPresent() { |
There was a problem hiding this comment.
That's not immediately clear from the code. Can we leave a comment indicating why we have a test with no assertions?
|
Kudos, SonarCloud Quality Gate passed!
|
|
A new generated diff is ready to view. |
Issue #
fixes #1098
Description of changes
f6502eb erroneously removed the explicit configuration settings
forcePathStyleandenableAcceleratefrom S3 client config. The intended change was to remove parsing these from CLI specific shared config.This PR also fixes finalize config to respect the environment/system property settings for
useArnRegionanddisableMrap(matching Java v2 for the system properties).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.