Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13320 )
Change subject: [backup] Factor out kudu-backup-tools module ...................................................................... Patch Set 1: (4 comments) Should we also include a log4j.properties in main/resources? Good question. If we don't, I think the default behavior is to log to stdout. http://gerrit.cloudera.org:8080/#/c/13320/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13320/1//COMMIT_MSG@12 PS1, Line 12: it’s > its Done http://gerrit.cloudera.org:8080/#/c/13320/1/java/gradle/shadow.gradle File java/gradle/shadow.gradle: http://gerrit.cloudera.org:8080/#/c/13320/1/java/gradle/shadow.gradle@53 PS1, Line 53: Our shaded library jars always try to pull in the slf4j api from : // kudu-client > For my own edification, what does this mean? The shaded jars depend on slf4 We have two "kinds" of jars in our build. Library jars and tool jars. A Library jar should shade all non-public API dependencies and leave any dependency that is a part of public API as a dependency. In the case of Slf4j, it is always public API so we don't want to shade it. However, for a tool jar which is only expected to be used as a standalone application and not a library, we want to included all needed dependencies. http://gerrit.cloudera.org:8080/#/c/13320/1/java/kudu-backup-tools/build.gradle File java/kudu-backup-tools/build.gradle: http://gerrit.cloudera.org:8080/#/c/13320/1/java/kudu-backup-tools/build.gradle@27 PS1, Line 27: dependencies { > As I understand it, this doesn't restrict the versions the tools can work w For Hadoop and Spark this is true. It is possible that there is some client/protocol breakage that isn't handled well because we are using a different version. This is the trade off of including the dependencies in the jar as opposed to building the classpath at runtime. Scala doesn't really matter since this is a tool that wont run with anything else on the classpath. That effectively makes Scala a library. Java has no extra concerns/considerations than any other jars we produce. http://gerrit.cloudera.org:8080/#/c/13320/1/java/kudu-backup/build.gradle File java/kudu-backup/build.gradle: http://gerrit.cloudera.org:8080/#/c/13320/1/java/kudu-backup/build.gradle@23 PS1, Line 23: compile(project(path: ":kudu-backup-tools")) { > Don't we need backup-tools to be a leaf dependency if we are shipping it as There is difference between what we publish and what we use. We always publish the shaded version, but for the sake of code re-use I am making this module depend on the non-shaded version. Gradle provides much more control around this than Maven has. -- To view, visit http://gerrit.cloudera.org:8080/13320 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ef2c21fbc31b11b20f0588b6de3cd4998b67443 Gerrit-Change-Number: 13320 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 14 May 2019 12:32:52 +0000 Gerrit-HasComments: Yes
