jackye1995 commented on code in PR #5684:
URL: https://github.com/apache/iceberg/pull/5684#discussion_r964255070


##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientFactories.java:
##########
@@ -86,99 +86,54 @@ private static AwsClientFactory loadClientFactory(String 
impl, Map<String, Strin
   }
 
   static class DefaultAwsClientFactory implements AwsClientFactory {
-
-    private String glueEndpoint;
-    private String s3Endpoint;
-    private String s3AccessKeyId;
-    private String s3SecretAccessKey;
-    private String s3SessionToken;
-    private Boolean s3PathStyleAccess;
-    private Boolean s3UseArnRegionEnabled;
-    private Boolean s3AccelerationEnabled;
-    private String dynamoDbEndpoint;
-    private String httpClientType;
-    private Boolean s3DualStackEnabled;
+    private AwsProperties awsProperties;
 
     DefaultAwsClientFactory() {}
 
     @Override
     public S3Client s3() {
       return S3Client.builder()
-          .httpClientBuilder(configureHttpClientBuilder(httpClientType))
-          .applyMutation(builder -> configureEndpoint(builder, s3Endpoint))
-          .dualstackEnabled(s3DualStackEnabled)
-          .serviceConfiguration(
-              S3Configuration.builder()
-                  .pathStyleAccessEnabled(s3PathStyleAccess)
-                  .useArnRegionEnabled(s3UseArnRegionEnabled)
-                  .accelerateModeEnabled(s3AccelerationEnabled)
-                  .build())
-          .credentialsProvider(
-              credentialsProvider(s3AccessKeyId, s3SecretAccessKey, 
s3SessionToken))
+          .applyMutation(awsProperties::applyHttpClientConfigurations)
+          .applyMutation(awsProperties::applyS3EndpointConfigurations)
+          .applyMutation(awsProperties::applyS3ServiceConfigurations)
+          .applyMutation(awsProperties::applyS3CredentialConfigurations)
           .build();
     }
 
     @Override
     public GlueClient glue() {
       return GlueClient.builder()
-          .httpClientBuilder(configureHttpClientBuilder(httpClientType))
-          .applyMutation(builder -> configureEndpoint(builder, glueEndpoint))
+          .applyMutation(awsProperties::applyHttpClientConfigurations)
+          .applyMutation(awsProperties::applyGlueEndpointConfigurations)
           .build();
     }
 
     @Override
     public KmsClient kms() {
       return KmsClient.builder()
-          .httpClientBuilder(configureHttpClientBuilder(httpClientType))
+          .applyMutation(awsProperties::applyHttpClientConfigurations)
           .build();
     }
 
     @Override
     public DynamoDbClient dynamo() {
       return DynamoDbClient.builder()
-          .httpClientBuilder(configureHttpClientBuilder(httpClientType))
-          .applyMutation(builder -> configureEndpoint(builder, 
dynamoDbEndpoint))
+          .applyMutation(awsProperties::applyHttpClientConfigurations)
+          .applyMutation(awsProperties::applyDynamoDbEndpointConfigurations)
           .build();
     }
 
     @Override
     public void initialize(Map<String, String> properties) {
-      this.glueEndpoint = properties.get(AwsProperties.GLUE_CATALOG_ENDPOINT);
-      this.s3Endpoint = properties.get(AwsProperties.S3FILEIO_ENDPOINT);
-      this.s3AccessKeyId = 
properties.get(AwsProperties.S3FILEIO_ACCESS_KEY_ID);
-      this.s3SecretAccessKey = 
properties.get(AwsProperties.S3FILEIO_SECRET_ACCESS_KEY);
-      this.s3SessionToken = 
properties.get(AwsProperties.S3FILEIO_SESSION_TOKEN);
-      this.s3PathStyleAccess =
-          PropertyUtil.propertyAsBoolean(
-              properties,
-              AwsProperties.S3FILEIO_PATH_STYLE_ACCESS,
-              AwsProperties.S3FILEIO_PATH_STYLE_ACCESS_DEFAULT);
-      this.s3UseArnRegionEnabled =
-          PropertyUtil.propertyAsBoolean(
-              properties,
-              AwsProperties.S3_USE_ARN_REGION_ENABLED,
-              AwsProperties.S3_USE_ARN_REGION_ENABLED_DEFAULT);
-      this.s3AccelerationEnabled =
-          PropertyUtil.propertyAsBoolean(
-              properties,
-              AwsProperties.S3_ACCELERATION_ENABLED,
-              AwsProperties.S3_ACCELERATION_ENABLED_DEFAULT);
-      this.s3DualStackEnabled =
-          PropertyUtil.propertyAsBoolean(
-              properties,
-              AwsProperties.S3_DUALSTACK_ENABLED,
-              AwsProperties.S3_DUALSTACK_ENABLED_DEFAULT);
+      this.awsProperties = new AwsProperties(properties);
 
       ValidationException.check(

Review Comment:
   I think we can move this validation to AwsProperties in its constructor



-- 
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]

Reply via email to