jackye1995 commented on a change in pull request #4175:
URL: https://github.com/apache/iceberg/pull/4175#discussion_r813165464
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/TestAssumeRoleAwsClientFactory.java
##########
@@ -58,7 +60,7 @@ public void before() {
roleName = UUID.randomUUID().toString();
iam = IamClient.builder()
.region(Region.AWS_GLOBAL)
- .httpClient(UrlConnectionHttpClient.create())
+ .httpClientBuilder(HTTP_CLIENT_BUILDER_DEFAULT)
Review comment:
I think there might be some race conditions, especially if this is
shared globally. Based on the documentation, services will inject necessary
configurations to the HTTP client builder, so at the same time, there might be
2 processes creating different clients, but if they share the same builder that
means the HTTP client produced will be e mix of the 2 service configs. This is
something we should better avoid. Given the fact that now each client has its
own HTTP client, reusing the builder is not really saving any resource since
the builder is ephemeral, so I think it is safer to use individual builders.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
