Savonitar commented on code in PR #29344:
URL: https://github.com/apache/flink/pull/29344#discussion_r4156430535
##########
flink-filesystems/flink-s3-fs-base/src/main/java/org/apache/flink/fs/s3/common/token/AbstractS3DelegationTokenProvider.java:
##########
@@ -87,22 +88,29 @@ public boolean delegationTokensRequired() {
public ObtainedDelegationTokens obtainDelegationTokens() throws Exception {
LOG.info("Obtaining session credentials token with access key: {}",
accessKey);
- AWSSecurityTokenService stsClient =
- AWSSecurityTokenServiceClientBuilder.standard()
- .withRegion(region)
- .withCredentials(
- new AWSStaticCredentialsProvider(
- new BasicAWSCredentials(accessKey,
secretKey)))
- .build();
- GetSessionTokenResult sessionTokenResult = stsClient.getSessionToken();
- Credentials credentials = sessionTokenResult.getCredentials();
- LOG.info(
- "Session credentials obtained successfully with access key: {}
expiration: {}",
- credentials.getAccessKeyId(),
- credentials.getExpiration());
+ final AWSSecurityTokenService stsClient = createStsClient();
+ // Preserve the acquisition failure if shutting down the client also
fails.
+ try (AutoCloseable ignored = stsClient::shutdown) {
Review Comment:
Thanks for the suggestion. I saw that risk, but decided to align with
existing behavior in
[NativeS3DelegationTokenProvider](https://github.com/apache/flink/blob/master/flink-filesystems/flink-s3-fs-native/src/main/java/org/apache/flink/fs/s3native/token/NativeS3DelegationTokenProvider.java#L128)
(propagates close() failures from its finally block). Happy to align that
issue in NativeS3DelegationTokenProvider as well as a follow up.
I think the issue is rare in this case, but I agree, losing valid tokens
during a cleanup isn't worth it.
I changed to best effort in last commit. Shutdown runs in finally, and a
RuntimeException from it is logged at WARN with the provider name.
I will fold this commit to shutdown commit after the review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
