HoustonPutman commented on a change in pull request #271:
URL: https://github.com/apache/solr/pull/271#discussion_r695851226



##########
File path: 
solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
##########
@@ -73,48 +80,51 @@
   // Error messages returned by S3 for a key not found.
   private static final Set<String> NOT_FOUND_CODES = Set.of("NoSuchKey", "404 
Not Found");
 
-  private final AmazonS3 s3Client;
+  private final S3Client s3Client;
 
   /** The S3 bucket where we read/write all data. */
   private final String bucketName;
 
   S3StorageClient(
-      String bucketName, String region, String proxyHost, int proxyPort, 
String endpoint) {
-    this(createInternalClient(region, proxyHost, proxyPort, endpoint), 
bucketName);
+      String bucketName,
+      String region,
+      String proxyUrl,
+      boolean proxyUseSystemSettings,
+      String endpoint) {
+    this(createInternalClient(region, proxyUrl, proxyUseSystemSettings, 
endpoint), bucketName);
   }
 
   @VisibleForTesting
-  S3StorageClient(AmazonS3 s3Client, String bucketName) {
+  S3StorageClient(S3Client s3Client, String bucketName) {
     this.s3Client = s3Client;
     this.bucketName = bucketName;
   }
 
-  private static AmazonS3 createInternalClient(
-      String region, String proxyHost, int proxyPort, String endpoint) {
-    ClientConfiguration clientConfig = new 
ClientConfiguration().withProtocol(Protocol.HTTPS);
-
+  private static S3Client createInternalClient(
+      String region, String proxyUrl, boolean proxyUseSystemSettings, String 
endpoint) {
+    ApacheHttpClient.Builder sdkHttpClientBuilder = ApacheHttpClient.builder();
     // If configured, add proxy
-    if (!StringUtils.isEmpty(proxyHost)) {
-      clientConfig.setProxyHost(proxyHost);
-      if (proxyPort > 0) {
-        clientConfig.setProxyPort(proxyPort);
-      }
+    ProxyConfiguration.Builder proxyConfigurationBuilder = 
ProxyConfiguration.builder();
+    if (!StringUtils.isEmpty(proxyUrl)) {
+      proxyConfigurationBuilder.endpoint(URI.create(proxyUrl));
+    } else {
+      
proxyConfigurationBuilder.useSystemPropertyValues(proxyUseSystemSettings);
     }
+    sdkHttpClientBuilder.proxyConfiguration(proxyConfigurationBuilder.build());
+    sdkHttpClientBuilder.useIdleConnectionReaper(false);

Review comment:
       I had to disable the idleConnectionReaper, because it caused "leaked" 
threads after the unit tests.

##########
File path: 
solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
##########
@@ -73,48 +80,51 @@
   // Error messages returned by S3 for a key not found.
   private static final Set<String> NOT_FOUND_CODES = Set.of("NoSuchKey", "404 
Not Found");
 
-  private final AmazonS3 s3Client;
+  private final S3Client s3Client;
 
   /** The S3 bucket where we read/write all data. */
   private final String bucketName;
 
   S3StorageClient(
-      String bucketName, String region, String proxyHost, int proxyPort, 
String endpoint) {
-    this(createInternalClient(region, proxyHost, proxyPort, endpoint), 
bucketName);
+      String bucketName,
+      String region,
+      String proxyUrl,
+      boolean proxyUseSystemSettings,
+      String endpoint) {
+    this(createInternalClient(region, proxyUrl, proxyUseSystemSettings, 
endpoint), bucketName);
   }
 
   @VisibleForTesting
-  S3StorageClient(AmazonS3 s3Client, String bucketName) {
+  S3StorageClient(S3Client s3Client, String bucketName) {
     this.s3Client = s3Client;
     this.bucketName = bucketName;
   }
 
-  private static AmazonS3 createInternalClient(
-      String region, String proxyHost, int proxyPort, String endpoint) {
-    ClientConfiguration clientConfig = new 
ClientConfiguration().withProtocol(Protocol.HTTPS);
-
+  private static S3Client createInternalClient(
+      String region, String proxyUrl, boolean proxyUseSystemSettings, String 
endpoint) {
+    ApacheHttpClient.Builder sdkHttpClientBuilder = ApacheHttpClient.builder();

Review comment:
       I think I had to switch to using the ApacheHttpClient for the S3Client, 
otherwise we wouldn't be able to use a proxy.
   
   The new ApacheHttpClient also lets us use system proxy settings now, so I've 
enabled that by default. But users can turn it off if they desire in the 
solr.xml..




-- 
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-unsubscr...@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to