You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/11/28 01:55:05 UTC
[kudu-CR] java: revisit maven shade configuration
Hello Mike Percy, Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/8661
to review the following change.
Change subject: java: revisit maven shade configuration
......................................................................
java: revisit maven shade configuration
This is a followup to [1], which reconfigured the Maven Shade Plugin to
apply excludes to all shaded jars, most notably on Guava classes. This
causes issues with kudu-flume-sink, which has a legitimate transitive
dependency on Guava through flume itself.
While investigating this issue I found that the `<include>*:*</include>`
configuration we commonly set for the shade plugin caused more
dependencies to be included than desired, for instance kudu-client-tools
was including hadoop-client, which was in provided scope.
The fix to both of these issues ended up being the same: only set the
exclusions where they should take effect in kudu-client, and be more
explicit about what jars should be included when shading in all other
modules.
I've done my best to make sure that the correct jars are still included
in the shaded jars using `jar -tf`.
[1]: https://github.com/apache/kudu/commit/5a258508f8d560f630512c237711a65cd137c6b3
Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
---
M java/kudu-client-tools/pom.xml
M java/kudu-client/pom.xml
M java/kudu-flume-sink/pom.xml
M java/kudu-spark-tools/pom.xml
M java/kudu-spark/pom.xml
M java/pom.xml
6 files changed, 65 insertions(+), 56 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8661/1
--
To view, visit http://gerrit.cloudera.org:8080/8661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Gerrit-Change-Number: 8661
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] java: revisit maven shade configuration
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Andrew Wong, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8661
to look at the new patch set (#3).
Change subject: java: revisit maven shade configuration
......................................................................
java: revisit maven shade configuration
This is a followup to [1], which reconfigured the Maven Shade Plugin to
apply excludes to all shaded jars, most notably on Guava classes. This
causes issues with kudu-flume-sink, which has a legitimate transitive
dependency on Guava through flume itself.
While investigating this issue I found that the `<include>*:*</include>`
configuration we commonly set for the shade plugin caused more
dependencies to be included than desired, for instance kudu-client-tools
was including hadoop-client, which was in provided scope.
The fix to both of these issues ended up being the same: only set the
exclusions where they should take effect in kudu-client, and be more
explicit about what jars should be included when shading in all other
modules.
I've done my best to make sure that the correct jars are still included
in the shaded jars using `jar -tf`.
[1]: https://github.com/apache/kudu/commit/5a258508f8d560f630512c237711a65cd137c6b3
Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
---
M java/kudu-client-tools/pom.xml
M java/kudu-client/pom.xml
M java/kudu-flume-sink/pom.xml
M java/kudu-spark-tools/pom.xml
M java/kudu-spark/pom.xml
M java/pom.xml
6 files changed, 69 insertions(+), 56 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8661/3
--
To view, visit http://gerrit.cloudera.org:8080/8661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Gerrit-Change-Number: 8661
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] java: revisit maven shade configuration
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8661 )
Change subject: java: revisit maven shade configuration
......................................................................
java: revisit maven shade configuration
This is a followup to [1], which reconfigured the Maven Shade Plugin to
apply excludes to all shaded jars, most notably on Guava classes. This
causes issues with kudu-flume-sink, which has a legitimate transitive
dependency on Guava through flume itself.
While investigating this issue I found that the `<include>*:*</include>`
configuration we commonly set for the shade plugin caused more
dependencies to be included than desired, for instance kudu-client-tools
was including hadoop-client, which was in provided scope.
The fix to both of these issues ended up being the same: only set the
exclusions where they should take effect in kudu-client, and be more
explicit about what jars should be included when shading in all other
modules.
I've done my best to make sure that the correct jars are still included
in the shaded jars using `jar -tf`.
[1]: https://github.com/apache/kudu/commit/5a258508f8d560f630512c237711a65cd137c6b3
Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Reviewed-on: http://gerrit.cloudera.org:8080/8661
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M java/kudu-client-tools/pom.xml
M java/kudu-client/pom.xml
M java/kudu-flume-sink/pom.xml
M java/kudu-spark-tools/pom.xml
M java/kudu-spark/pom.xml
M java/pom.xml
6 files changed, 69 insertions(+), 56 deletions(-)
Approvals:
Kudu Jenkins: Verified
Mike Percy: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Gerrit-Change-Number: 8661
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] java: revisit maven shade configuration
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8661 )
Change subject: java: revisit maven shade configuration
......................................................................
Patch Set 2:
I hacked up a script to validate shaded and unshaded dependencies and came up with this script and set of patterns: https://gerrit.cloudera.org/c/8662/
One thing I noticed is that yetus is not shaded. Is that what we want?
--
To view, visit http://gerrit.cloudera.org:8080/8661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Gerrit-Change-Number: 8661
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 28 Nov 2017 03:14:49 +0000
Gerrit-HasComments: No
[kudu-CR] java: revisit maven shade configuration
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8661 )
Change subject: java: revisit maven shade configuration
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Gerrit-Change-Number: 8661
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 28 Nov 2017 10:09:15 +0000
Gerrit-HasComments: No
[kudu-CR] java: revisit maven shade configuration
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8661 )
Change subject: java: revisit maven shade configuration
......................................................................
Patch Set 3:
> I hacked up a script to validate shaded and unshaded dependencies and came up with this script and set of patterns: https://gerrit.cloudera.org/c/8662/
Nice, I think this can help us not regress on this.
> One thing I noticed is that yetus is not shaded. Is that what we want?
Yeah, I think our annotations are technically part of the public API, since they aren't shaded.
--
To view, visit http://gerrit.cloudera.org:8080/8661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f02a37a5a6683dbbb226a1faf96647a112d7c0d
Gerrit-Change-Number: 8661
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 28 Nov 2017 04:33:05 +0000
Gerrit-HasComments: No