jackye1995 commented on code in PR #5684:
URL: https://github.com/apache/iceberg/pull/5684#discussion_r971329690
##########
aws/src/main/java/org/apache/iceberg/aws/AssumeRoleAwsClientFactory.java:
##########
@@ -34,133 +31,92 @@
import software.amazon.awssdk.services.sts.StsClient;
import
software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider;
import software.amazon.awssdk.services.sts.model.AssumeRoleRequest;
-import software.amazon.awssdk.services.sts.model.Tag;
public class AssumeRoleAwsClientFactory implements AwsClientFactory {
-
- private String roleArn;
- private String externalId;
- 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(this::applyAssumeRoleConfigurations)
+ .applyMutation(awsProperties::applyHttpClientConfigurations)
+ .applyMutation(awsProperties::applyS3EndpointConfigurations)
+ .applyMutation(awsProperties::applyS3ServiceConfigurations)
.build();
}
@Override
public GlueClient glue() {
- return GlueClient.builder().applyMutation(this::configure).build();
+ return GlueClient.builder()
+ .applyMutation(this::applyAssumeRoleConfigurations)
+ .applyMutation(awsProperties::applyHttpClientConfigurations)
+ .build();
}
@Override
public KmsClient kms() {
- return KmsClient.builder().applyMutation(this::configure).build();
+ return KmsClient.builder()
+ .applyMutation(this::applyAssumeRoleConfigurations)
+ .applyMutation(awsProperties::applyHttpClientConfigurations)
+ .build();
}
@Override
public DynamoDbClient dynamo() {
return DynamoDbClient.builder()
- .applyMutation(this::configure)
- .applyMutation(builder ->
AwsClientFactories.configureEndpoint(builder, dynamoDbEndpoint))
+ .applyMutation(this::applyAssumeRoleConfigurations)
+ .applyMutation(awsProperties::applyHttpClientConfigurations)
+ .applyMutation(awsProperties::applyDynamoDbEndpointConfigurations)
.build();
}
@Override
public void initialize(Map<String, String> properties) {
- this.roleArn = properties.get(AwsProperties.CLIENT_ASSUME_ROLE_ARN);
+ this.awsProperties = new AwsProperties(properties);
Preconditions.checkNotNull(
- roleArn, "Cannot initialize AssumeRoleClientConfigFactory with null
role ARN");
- this.timeout =
- PropertyUtil.propertyAsInt(
- properties,
- AwsProperties.CLIENT_ASSUME_ROLE_TIMEOUT_SEC,
- AwsProperties.CLIENT_ASSUME_ROLE_TIMEOUT_SEC_DEFAULT);
- this.externalId =
properties.get(AwsProperties.CLIENT_ASSUME_ROLE_EXTERNAL_ID);
-
- this.region = properties.get(AwsProperties.CLIENT_ASSUME_ROLE_REGION);
+ awsProperties.clientAssumeRoleArn(),
+ "Cannot initialize AssumeRoleClientConfigFactory with null role ARN");
Preconditions.checkNotNull(
- region, "Cannot initialize AssumeRoleClientConfigFactory with null
region");
+ awsProperties.clientAssumeRoleRegion(),
+ "Cannot initialize AssumeRoleClientConfigFactory with null region");
- this.s3Endpoint = properties.get(AwsProperties.S3FILEIO_ENDPOINT);
- this.tags = toTags(properties);
- this.s3UseArnRegionEnabled =
- PropertyUtil.propertyAsBoolean(
- properties,
- AwsProperties.S3_USE_ARN_REGION_ENABLED,
- AwsProperties.S3_USE_ARN_REGION_ENABLED_DEFAULT);
- this.dynamoDbEndpoint = properties.get(AwsProperties.DYNAMODB_ENDPOINT);
- this.httpClientType =
- PropertyUtil.propertyAsString(
- properties, AwsProperties.HTTP_CLIENT_TYPE,
AwsProperties.HTTP_CLIENT_TYPE_DEFAULT);
- }
-
- protected <T extends AwsClientBuilder & AwsSyncClientBuilder> T configure(T
clientBuilder) {
- AssumeRoleRequest request =
+ this.assumeRoleRequest =
AssumeRoleRequest.builder()
- .roleArn(roleArn)
+ .roleArn(awsProperties.clientAssumeRoleArn())
.roleSessionName(genSessionName())
- .durationSeconds(timeout)
- .externalId(externalId)
- .tags(tags)
+ .durationSeconds(awsProperties.clientAssumeRoleTimeoutSec())
+ .externalId(awsProperties.clientAssumeRoleExternalId())
+ .tags(awsProperties.stsClientAssumeRoleTags())
.build();
+ }
+ protected <T extends AwsClientBuilder & AwsSyncClientBuilder> T
applyAssumeRoleConfigurations(
+ T clientBuilder) {
clientBuilder.credentialsProvider(
Review Comment:
can chain the methods at L95-104 and directly return the builder
--
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]
