jackye1995 commented on code in PR #5684:
URL: https://github.com/apache/iceberg/pull/5684#discussion_r962342233
##########
aws/src/main/java/org/apache/iceberg/aws/AssumeRoleAwsClientFactory.java:
##########
@@ -43,17 +43,16 @@ public class AssumeRoleAwsClientFactory implements
AwsClientFactory {
private Set<Tag> tags;
private int timeout;
private String region;
- private String s3Endpoint;
- private boolean s3UseArnRegionEnabled;
- private String dynamoDbEndpoint;
- private String httpClientType;
+ private AwsProperties awsProperties;
+ private AssumeRoleRequest assumeRoleRequest;
@Override
public S3Client s3() {
return S3Client.builder()
.applyMutation(this::configure)
- .applyMutation(builder ->
AwsClientFactories.configureEndpoint(builder, s3Endpoint))
- .serviceConfiguration(s ->
s.useArnRegionEnabled(s3UseArnRegionEnabled).build())
+ .applyMutation(
+ builder -> AwsClientFactories.configureEndpoint(builder,
awsProperties.s3Endpoint()))
Review Comment:
this method can also be moved to
`awsProperties::applyEndpointConfigurations`, and we can set the endpoint based
on the instance type, for example `if (builder instanceof S3ClientBuilder) {
... }`
--
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]
