ahmarsuhail commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1472858212
##########
hadoop-project/pom.xml:
##########
@@ -187,7 +187,7 @@
<make-maven-plugin.version>1.0-beta-1</make-maven-plugin.version>
<surefire.fork.timeout>900</surefire.fork.timeout>
<aws-java-sdk.version>1.12.599</aws-java-sdk.version>
- <aws-java-sdk-v2.version>2.23.5</aws-java-sdk-v2.version>
Review Comment:
cut, SDK upgrade needs to happen separately
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
Review Comment:
@steveloughran do you have any advice here? I think we should do what we did
for Client Side Encryption, have this S3 access grants jar as optional, and
create a new client factory which will add the S3 access grants plugin.
If there are other plugins that we want to add in the future, this new
client factory can be generalised for that.
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
Review Comment:
this should go in hadoop-project, and then set the dependency here. We
should probably also use `provided` scope, so the jar is optional.
See this PR, which added the client side encryption for example
https://github.com/apache/hadoop/pull/6164 and
[this](https://github.com/apache/hadoop/pull/6164/files#diff-c76d380f28cd282404a2b7110a6ea76bf2edd7277ed09639a2af594171b07efaR53)
class which checks if the class exists
##########
LICENSE-binary:
##########
@@ -363,7 +363,7 @@ org.objenesis:objenesis:2.6
org.xerial.snappy:snappy-java:1.1.10.4
org.yaml:snakeyaml:2.0
org.wildfly.openssl:wildfly-openssl:1.1.3.Final
-software.amazon.awssdk:bundle:jar:2.23.5
Review Comment:
this shouldn't be here. it's already part of your SDK upgrade PR so cut from
here
--
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]