-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19343: Include hadoop-gcp in distro. #7877
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
This will need a rebase after #7874 gets committed. |
a5bf1f6
to
793a50d
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@@ -351,7 +351,6 @@ | |||
</shadedPattern> | |||
</relocation> | |||
</relocations> | |||
<shadedArtifactAttached>true</shadedArtifactAttached> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we removing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have two options:
- shadedArtifactAttached=true means a separate shaded jar artifact will be attached to the build. Clients can depend on it by specifying
<classifier>shaded</classifier>
in the dependency declaration. - shadedArtifactAttached=false (the default, hence removed in this patch) means the shaded jar completely replaces the published artifact. I think this is what we want for hadoop-gcp, given the tight coupling to specific shaded versions of things like Guava and gRPC. There wouldn't be much value in the non-shaded artifacts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #7877 Signed-off-by: Ayush Saxena <[email protected]> Reviewed-by: Arunkumar Chacko <[email protected]>
I merged this to the feature branch. Thank you for the reviews, @arunkumarchacko and @ayushtkn . |
Closes #7877 Signed-off-by: Ayush Saxena <[email protected]> Reviewed-by: Arunkumar Chacko <[email protected]>
Closes apache#7877 Signed-off-by: Ayush Saxena <[email protected]> Reviewed-by: Arunkumar Chacko <[email protected]>
Description of PR
Include hadoop-gcp in hadoop-tools-dist, so that it makes it into the final release tarball.
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?