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



##########
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:
       Yeah let's stick with sync for now. I'm fine switching to async in the 
future, but I'd rather we limit the scope of this ticket.
   
   The only non-supported option between apache and netty is that we are using 
the `useSystemPropertyValues` option provided by apache. If we move to netty in 
the future, we would just need to make sure to check the system properties 
ourselves and set the appropriate proxy options manually for the user.
   
   Otherwise we can convert `proxyUrl` to `host`, `port` and `scheme` ourselves 
very easily. (another difference between apache and netty)




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