steveloughran commented on a change in pull request #970: HADOOP-16371: Option to disable GCM for SSL connections when running on Java 8 URL: https://github.com/apache/hadoop/pull/970#discussion_r295216004
########## File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java ########## @@ -34,6 +34,7 @@ import com.amazonaws.services.s3.model.AmazonS3Exception; import com.amazonaws.services.s3.model.MultiObjectDeleteException; import com.amazonaws.services.s3.model.S3ObjectSummary; +import com.amazonaws.thirdparty.apache.http.conn.ssl.SSLConnectionSocketFactory; Review comment: I worry about this as it then ensures that the s3a code will only load when the shaded aws JAR is on the CP; people won't be able to switch to the unshaded JAR with the unshaded httpclient code. I'm also trying to move off adding more stuff to the S3AUtils class as its become a merge conflict point with no real structure: every patch adds something to it. See [Refactoring S3A](https://github.com/steveloughran/engineering-proposals/blob/master/refactoring-s3a.md) for my thoughts there. As this is adding new network setup, I'm going propose * putting this into a new class in o.a.h.fs.s3a.impl; -something like "NetworkBinding", where we can add more stuff later on * whatever is done to set up the AWS networking is done through reflection; if the shaded class can't be found on the CP, then just skip trying to configure it. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
