Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19146 )
Change subject: IMPALA-11667: Clean up Java dependency exclusions ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/19146/5/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/19146/5/fe/pom.xml@a122 PS5, Line 122: : : Can you keep this comment? http://gerrit.cloudera.org:8080/#/c/19146/3/java/pom.xml File java/pom.xml: http://gerrit.cloudera.org:8080/#/c/19146/3/java/pom.xml@255 PS3, Line 255: <dependencies> : <!-- Provided by reload4j instead. --> : <dependency> : <groupId>log4j</groupId> : <artifactId>log4j</artifactId> : <version>1.2.17</version> : <scope>provided</scope> : </dependency> : : <!-- Provided by our own s3a-aws-sdk shaded jar. --> : <dependency> : <groupId>com.amazonaws</groupId> : <artifactId>aws-java-sdk-bundle</artifactId> : <version>1.12.132</version> : <scope>provided</scope> : </dependency> : </dependencies> I think this is a problem for the shaded s3 jar. The minimal shaded s3 jar has a dependency on hadoop-aws and gets aws-java-sdk-bundle as a result of that dependency. That ensures that our version matches hadoop's version. The provided dependency means that hadoop-aws doesn't pull in aws-java-sdk-bundle and the shaded jar is empty. Other thought: Using provided as a replacement for exclusions is pretty subtle. It would be ok if these remained as exclusions. -- To view, visit http://gerrit.cloudera.org:8080/19146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I424a175135855dcbd38ae432ea111cca5f562633 Gerrit-Change-Number: 19146 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Mon, 17 Oct 2022 23:57:16 +0000 Gerrit-HasComments: Yes
