You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/08/25 15:12:28 UTC

[GitHub] [solr] HoustonPutman commented on a change in pull request #271: SOLR-15599: Upgrade aws sdk

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

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



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