You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Kontinuation (via GitHub)" <gi...@apache.org> on 2023/09/29 01:49:35 UTC

[GitHub] [iceberg] Kontinuation opened a new pull request, #8677: Fix java.lang.IllegalStateException: Connection pool shut down when using web identity token file to authenticate

Kontinuation opened a new pull request, #8677:
URL: https://github.com/apache/iceberg/pull/8677

   Fixes https://github.com/apache/iceberg/issues/8601
   
   It uses `DefaultCredentialsProvider.builder().build()` instead of `DefaultCredentialsProvider.create()` to avoid sharing the same credentials provider across multiple clients.
   
   This PR changes 3 places calling `DefaultCredentialsProvider.create()`, and adds one test for `AwsCredentialsProvider.credentialsProvider`. The other two are deprecated so no tests were added for them.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Kontinuation commented on a diff in pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on code in PR #8677:
URL: https://github.com/apache/iceberg/pull/8677#discussion_r1342038887


##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientFactories.java:
##########
@@ -226,7 +226,7 @@ static AwsCredentialsProvider credentialsProvider(
             AwsSessionCredentials.create(accessKeyId, secretAccessKey, sessionToken));
       }
     } else {
-      return DefaultCredentialsProvider.create();
+      return DefaultCredentialsProvider.builder().build();

Review Comment:
   Comment added as requested.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #8677:
URL: https://github.com/apache/iceberg/pull/8677#issuecomment-1742304383

   thanks @Kontinuation for the root cause analysis and the fix. Thanks @dramaticlly @amogh-jahagirdar @danielcweeks for reviews 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu merged pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu merged PR #8677:
URL: https://github.com/apache/iceberg/pull/8677


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on a diff in pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #8677:
URL: https://github.com/apache/iceberg/pull/8677#discussion_r1342190477


##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientFactories.java:
##########
@@ -226,7 +226,7 @@ static AwsCredentialsProvider credentialsProvider(
             AwsSessionCredentials.create(accessKeyId, secretAccessKey, sessionToken));
       }
     } else {
-      return DefaultCredentialsProvider.create();
+      return DefaultCredentialsProvider.builder().build();

Review Comment:
   @Kontinuation if we add the comment, let's add it to all the places



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Kontinuation commented on a diff in pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on code in PR #8677:
URL: https://github.com/apache/iceberg/pull/8677#discussion_r1342208899


##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientFactories.java:
##########
@@ -226,7 +226,7 @@ static AwsCredentialsProvider credentialsProvider(
             AwsSessionCredentials.create(accessKeyId, secretAccessKey, sessionToken));
       }
     } else {
-      return DefaultCredentialsProvider.create();
+      return DefaultCredentialsProvider.builder().build();

Review Comment:
   done



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #8677:
URL: https://github.com/apache/iceberg/pull/8677#issuecomment-1741141528

   @Kontinuation thx for the fix. LGTM.  I updated the subject. will leave it for a couple of days for others to 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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #8677: AWS: avoid static global credentials provider which doesn't play well with lifecycle management

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #8677:
URL: https://github.com/apache/iceberg/pull/8677#discussion_r1341987133


##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientFactories.java:
##########
@@ -226,7 +226,7 @@ static AwsCredentialsProvider credentialsProvider(
             AwsSessionCredentials.create(accessKeyId, secretAccessKey, sessionToken));
       }
     } else {
-      return DefaultCredentialsProvider.create();
+      return DefaultCredentialsProvider.builder().build();

Review Comment:
   Nit: Maybe have an inline comment along the lines of "Create a new credential provider for each client"



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org