-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19343: Manage hadoop-gcp Guava version directly in its pom.xml and mark exclusion in hadoop-tools-dist. #7904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hello @slfan1989 and @pan3793 . This is a reattempt of #7883, which I had to revert. The key difference here is the exclusion in hadoop-tools-dist/pom.xml, so that when the distro module is built, there are no dependency convergence errors. |
|
💔 -1 overall
This message was automatically generated. |
@cnauroth I checked the build logs, and there are still some dependency issues that need to be resolved. |
|
I lean towards unifying the Guava version in the whole project, supposing the assumption is true. |
Additional information In |
|
@slfan1989 AFAIK, third-party libs, e.g. Curator, require vanilla Guava classes in runtime. |
|
Hi @slfan1989 . @pan3793 is correct. All of the hadoop-gcp code itself is using the hadoop-thirdparty shaded Guava: The specific issue here is about the Guava used by the GCS SDK. We don't have a way to rewire its imports to point at our third-party. Also, we've seen historically that the GCS SDK can be particular about the version of Guava it uses. Historically, the original codebase from the Google repo has shaded this to guarantee a matching version and also isolate Hadoop clients from unexpected changes in the Guava version. |
@cnauroth Thank you for the clarification! I have no further questions about this PR. I'm curious why the Hadoop main project hasn't upgraded the Guava version. It seems 27.0-jre is from 2018. |
… and mark exclusion in hadoop-tools-dist. Closes apache#7904
ffbf3f9 to
93c3d2a
Compare
|
@slfan1989 , thanks for the review! The reason for the last Yetus failure was that I didn't have the right match for the Guava version number used by the GCS SDK. I pushed up an update. I also added more comments to explain what's going on for future maintainers. I'll wait for a clean Yetus run before committing. Regarding the old Guava version, I honestly don't remember why it's there, considering we now have hadoop-thirdparty. Maybe it was an assumption that old client projects had come to rely on it as a transitive dependency? There has been a policy that exposed dependencies are treated as public/stable: Maybe 3.5.0 is an acceptable version boundary to remove this. I've been planning to start a separate discussion. |
|
💔 -1 overall
This message was automatically generated. |
|
The last -1s are for lack of tests (not relevant) and root build failing (due to dependency convergence problem that this patch is fixing). I'll plan on committing to the feature branch later. |
… and mark exclusion in hadoop-tools-dist. Closes #7904 Signed-off-by: Shilun Fan <slfan1989@apache.org>
|
Thanks again everyone! This is merged into the feature branch. |
… and mark exclusion in hadoop-tools-dist. Closes apache#7904 Signed-off-by: Shilun Fan <slfan1989@apache.org>
Description of PR
HADOOP-19343: Manage hadoop-gcp Guava version directly in its pom.xml and mark exclusion in hadoop-tools-dist.
How was this patch tested?
mvn -Pdist -Dtar -DskipTests clean package. Confirmed no dependency convergence errors.hadoop fscommands against a GCS bucket.mvn clean verifyin hadoop-gcp to confirm all integration tests pass.For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?