[GitHub] [iceberg] JonasJ-ap commented on a diff in pull request #5787: AWS: Add socket connection timeout for Apache Http Builder

[GitBox <gi...@apache.org>]

JonasJ-ap commented on code in PR #5787:
URL: https://github.com/apache/iceberg/pull/5787#discussion_r979074174


##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsProperties.java:
##########
@@ -204,4 +209,139 @@ public void testS3FileIoSessionCredentialsConfiguration() {
         "secret",
         capturedAwsCredentialsProvider.resolveCredentials().secretAccessKey());
   }
+
+  @Test
+  public void testUrlHttpClientConfiguration() {
+    Map<String, String> properties = Maps.newHashMap();
+    properties.put(AwsProperties.HTTP_CLIENT_TYPE, "urlconnection");
+    AwsProperties awsProperties = new AwsProperties(properties);
+    S3ClientBuilder mockS3ClientBuilder = Mockito.mock(S3ClientBuilder.class);
+    ArgumentCaptor<SdkHttpClient.Builder> httpClientBuilderCaptor =
+        ArgumentCaptor.forClass(SdkHttpClient.Builder.class);
+
+    awsProperties.applyHttpClientConfigurations(mockS3ClientBuilder);
+    Mockito.verify(mockS3ClientBuilder).httpClientBuilder(httpClientBuilderCaptor.capture());
+    SdkHttpClient.Builder capturedHttpClientBuilder = httpClientBuilderCaptor.getValue();
+
+    Assert.assertTrue(
+        "Should use url connection http client",
+        capturedHttpClientBuilder instanceof UrlConnectionHttpClient.Builder);
+  }
+
+  @Test
+  public void testApacheHttpClientConfiguration() {
+    Map<String, String> properties = Maps.newHashMap();
+    properties.put(AwsProperties.HTTP_CLIENT_TYPE, "apache");
+    AwsProperties awsProperties = new AwsProperties(properties);
+    S3ClientBuilder mockS3ClientBuilder = Mockito.mock(S3ClientBuilder.class);
+    ArgumentCaptor<SdkHttpClient.Builder> httpClientBuilderCaptor =
+        ArgumentCaptor.forClass(SdkHttpClient.Builder.class);
+
+    awsProperties.applyHttpClientConfigurations(mockS3ClientBuilder);
+    Mockito.verify(mockS3ClientBuilder).httpClientBuilder(httpClientBuilderCaptor.capture());
+    SdkHttpClient.Builder capturedHttpClientBuilder = httpClientBuilderCaptor.getValue();
+    Assert.assertTrue(
+        "Should use apache http client",
+        capturedHttpClientBuilder instanceof ApacheHttpClient.Builder);
+  }
+
+  @Test
+  public void testInvalidHttpClientType() {
+    Map<String, String> properties = Maps.newHashMap();
+    properties.put(AwsProperties.HTTP_CLIENT_TYPE, "test");
+    AwsProperties awsProperties = new AwsProperties(properties);
+    S3ClientBuilder s3ClientBuilder = S3Client.builder();
+
+    AssertHelpers.assertThrows(
+        "should not support http client types other than urlconnection and apache",
+        IllegalArgumentException.class,
+        "Unrecognized HTTP client type",
+        () -> awsProperties.applyHttpClientConfigurations(s3ClientBuilder));
+  }
+
+  @Test
+  public void testApacheConnectionSocketTimeoutConfiguration() {

Review Comment:
   These are newly added unit tests for the timeout configurations



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org