Fix usage of unicode in bucket names of s3 presigner #487
Conversation
…ters needed for unicode bucket names in s3. Update codegen to specify values based on service. Update S3 service tests for unicode bucket names.
|
A new generated diff is ready to view: __generated-main...__generated-fix-s3-unicode-presigners |
1 similar comment
|
A new generated diff is ready to view: __generated-main...__generated-fix-s3-unicode-presigners |
|
A new generated diff is ready to view: __generated-main...__generated-fix-s3-unicode-presigners |
|
A new generated diff is ready to view: __generated-main...__generated-fix-s3-unicode-presigners |
| body = ByteStream.fromString(contents) | ||
| } | ||
| testKeyNames.reversed().forEach { keyName -> | ||
| println("testing $keyName") |
| CrtHttpEngine().use { engine -> | ||
| val httpClient = sdkHttpClient(engine) | ||
| CrtHttpEngine().use { engine -> | ||
| val httpClient = sdkHttpClient(engine) |
There was a problem hiding this comment.
style/suggestion
sdkHttpClient will close the engine by default so this could be written as:
sdkHttpClient(CrtHttpEngine()).use { httpClient ->
...
}There was a problem hiding this comment.
I get the following error when changing to your suggested code: Unresolved reference. None of the following candidates is applicable because of receiver type mismatch: public inline fun <T : Closeable?, R> TypeVariable(T).use(block: (TypeVariable(T)) -> TypeVariable(R)): TypeVariable(R) defined in kotlin.io
I'm not seeing anywhere where SdkHttpClient implements Closable..
There was a problem hiding this comment.
Hmm indeed it does not implement Closeable yet has a close() method. Probably a miss (or likely because SdkHttpClient predates our addition of Closeable). ignore for now then
|
SonarCloud Quality Gate failed.
|
|
A new generated diff is ready to view: __generated-main...__generated-fix-s3-unicode-presigners |
3 participants
Issue #
#485
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.