[FLINK-36975][Connectors/AWS] Fix the AWS Config Option AWS_ROLE_CREDENTIALS_PROVIDER_OPTION to pick the right key#196
[FLINK-36975][Connectors/AWS] Fix the AWS Config Option AWS_ROLE_CREDENTIALS_PROVIDER_OPTION to pick the right key#196nageshvh wants to merge 2 commits intoapache:mainfrom
Conversation
…ENTIALS_PROVIDER_OPTION to pick the right key
leekeiabstraction left a comment
There was a problem hiding this comment.
Thank you for the PR!
… for AWS Kinesis Stream sink
| * mock clients. | ||
| */ | ||
| @Internal | ||
| interface KinesisClientProvider { |
There was a problem hiding this comment.
Can this also extend SdkAutoCloseable so it's inline with the other resource impls?
There was a problem hiding this comment.
not sure what has happened here but this PR didn't change this file. This PR - #201 already addresses your comments, please check - @mohsenrezaeithe
| if (kinesisClientProvider != null) { | ||
| // Use the provided client for testing | ||
| this.httpClient = null; | ||
| this.kinesisClient = kinesisClientProvider.get(); | ||
| } else { | ||
| // Create a new client as before | ||
| this.httpClient = AWSGeneralUtil.createAsyncHttpClient(kinesisClientProperties); | ||
| this.kinesisClient = buildClient(kinesisClientProperties, this.httpClient); | ||
| } |
There was a problem hiding this comment.
Nit: These can be pushed to their own constructors so you don't have to pass in null or do a null check.
| if (kinesisClientProvider != null) { | ||
| kinesisClientProvider.close(); | ||
| } else { | ||
| AWSGeneralUtil.closeResources(httpClient, kinesisClient); |
There was a problem hiding this comment.
IIUC, by having the provider extend SdkAutoCloseable, you can pass it as the last parameter to the util for resource cleanup.
There was a problem hiding this comment.
not sure what has happened here but this PR didn't change this file. This PR - #201 already addresses your comments, please check - @mohsenrezaeithe
Purpose of the change
[FLINK-36975][Connectors/AWS] Fix the AWS Config Option AWS_ROLE_CREDENTIALS_PROVIDER_OPTION to pick the right key
https://issues.apache.org/jira/browse/FLINK-36975
Verifying this change
This change added tests and can be verified as follows:
Significant changes
(Please check any boxes [x] if the answer is "yes". You can first publish the PR and check them afterwards, for convenience.)
@Public(Evolving))