jackye1995 commented on a change in pull request #4371:
URL: https://github.com/apache/iceberg/pull/4371#discussion_r832706033



##########
File path: aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java
##########
@@ -208,6 +208,27 @@
    */
   public static final String CLIENT_FACTORY = "client.factory";
 
+  /**
+   * The type of {@link software.amazon.awssdk.http.SdkHttpClient} 
implementation used by {@link AwsClientFactory}
+   * If set, all AWS clients will use this specified HTTP client.
+   * If not set, CLIENT_HTTP_TYPE_DEFAULT will be used.
+   * For specific types supported, see CLIENT_HTTP_TYPE_* defined below.
+   */
+  public static final String CLIENT_HTTP_TYPE = "client.http.type";

Review comment:
       I think the issue I was having with the original name was that it's a 
"client type", but there are 2 "client"s in the config name. `http.client.type` 
sounds good to me, it also matches the AWS class path for these clients, for 
example `software.amazon.awssdk.http.apache`. We don't really need the top 
level `client.` config, because it is configuring one level deeper.
   
   I also thought about the potential configs in the future we want to add, 
such as connection and socket timeout, common configs can be after the same 
prefix, such as `http.client.connection-timeout`, and client specific ones can 
be for example `http.apache.connection-acquisition-timeout`.
   
   @xiaoxuandev what do you think?




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