[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170426903 ## File path: assemble/pom.xml ## @@ -256,6 +234,28 @@ + +org.apache.maven.plugins +maven-dependency-plugin Review comment: Done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170426906 ## File path: assemble/bin/bootstrap_config.sh ## @@ -383,6 +394,25 @@ elif [[ "${HADOOP_VERSION}" == "IOP4.1" ]]; then sed -e 's///-->/' \ "${CONF_DIR}/$ACCUMULO_SITE" > temp + sed -e 's///-->/' \ + "${CONF_DIR}/$ACCUMULO_SITE" > temp + mv temp "${CONF_DIR}/$ACCUMULO_SITE" +elif [[ "${HADOOP_VERSION}" == "3" ]]; then + sed -e 's///-->/' \ + "${CONF_DIR}/$ACCUMULO_SITE" > temp + sed -e 's///-->/' \ + "${CONF_DIR}/$ACCUMULO_SITE" > temp Review comment: All of these were fixed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170426869 ## File path: assemble/pom.xml ## @@ -323,5 +323,67 @@ + + hadoop2 + + + + !hadoop.profile + + + + commons-math3,commons-vfs2,gson,guava,htrace-core,javax.servlet-api,jcommander,jetty-http,jetty-io,jetty-security,jetty-server,jetty-servlet,jetty-util,jline,libthrift,protobuf-java,slf4j-api,slf4j-log4j12 + src/main/assemblies/binary-release.xml + + + + hadoop3 + + + hadoop.profile + 3 + + + + commons-cli,commons-codec,commons-collections,commons-configuration,commons-io,commons-lang,commons-logging,commons-math3,commons-vfs2,gson,guava,htrace-core,htrace-core4,javax.servlet-api,jcommander,jetty-http,jetty-io,jetty-security,jetty-server,jetty-servlet,jetty-util,jline,libthrift,protobuf-java,slf4j-api,slf4j-log4j12 + src/main/assemblies/binary-release-hadoop3.xml + + Review comment: Turns out we don't need any additions. The only artifact with an entry to NOTICE is about a file that's only in the test jar (and thus we don't need to include). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170425669 ## File path: assemble/pom.xml ## @@ -323,5 +323,67 @@ + + hadoop2 + + + + !hadoop.profile + + + + commons-math3,commons-vfs2,gson,guava,htrace-core,javax.servlet-api,jcommander,jetty-http,jetty-io,jetty-security,jetty-server,jetty-servlet,jetty-util,jline,libthrift,protobuf-java,slf4j-api,slf4j-log4j12 + src/main/assemblies/binary-release.xml + + + + hadoop3 + + + hadoop.profile + 3 + + + + commons-cli,commons-codec,commons-collections,commons-configuration,commons-io,commons-lang,commons-logging,commons-math3,commons-vfs2,gson,guava,htrace-core,htrace-core4,javax.servlet-api,jcommander,jetty-http,jetty-io,jetty-security,jetty-server,jetty-servlet,jetty-util,jline,libthrift,protobuf-java,slf4j-api,slf4j-log4j12 + src/main/assemblies/binary-release-hadoop3.xml + + Review comment: Sadly, probably. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170425661 ## File path: core/pom.xml ## @@ -282,5 +277,50 @@ + + hadoop2 + + + Review comment: I will change the comment for you This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170425499 ## File path: pom.xml ## @@ -128,7 +128,9 @@ 3.0.5 1 +2 Review comment: > when we had a 1 and 2 profile, we had to create a third hadoop default profile I remembered this, but didn't look back in history to figure it out again. Let me re-add that. Would be better than having `-Dhadoop.profile=2` oddly not work. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170408222 ## File path: server/gc/pom.xml ## @@ -87,4 +83,40 @@ test + + + hadoop2 + + + hadoop.profile + 2 + + + + + org.apache.hadoop + hadoop-client + + + + + hadoop3 + + + hadoop.profile + 3 + + + + + org.apache.hadoop + hadoop-client-api + + + org.apache.hadoop + hadoop-client-runtime Review comment: The only thing in the hadoop-client-runtime jar is relocated dependency classes -- no Hadoop code at all. So, no, it's presently 100% guaranteed that we're not using anything from it. Runtime sounds correct. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars
joshelser commented on a change in pull request #332: ACCUMULO-4826 Use the hadoop3 shaded jars URL: https://github.com/apache/accumulo/pull/332#discussion_r170407578 ## File path: assemble/pom.xml ## @@ -323,5 +283,147 @@ + + hadoop2 + + + hadoop.profile + 2 + + + + + +org.apache.maven.plugins +maven-dependency-plugin + + + +generate-dependencies-raw + + list + +compile + + ${project.build.directory}/dependencies.raw.txt + false + true + + commons-math3,commons-vfs2,gson,guava,htrace-core,javax.servlet-api,jcommander,jetty-http,jetty-io,jetty-security,jetty-server,jetty-servlet,jetty-util,jline,libthrift,protobuf-java,slf4j-api,slf4j-log4j12 + true + + + + + +org.apache.maven.plugins +maven-assembly-plugin + + +binary-assembly + + single + +package + + + src/main/assemblies/binary-release.xml + + + + + + + + + + hadoop3 + + + hadoop.profile + 3 + + + + + + commons-cli + commons-cli + + + commons-codec + commons-codec + + + commons-collections + commons-collections + + + commons-configuration + commons-configuration + + + commons-io + commons-io + + + commons-lang + commons-lang + + + commons-logging + commons-logging + + + org.apache.htrace + htrace-core4 + + + + + +org.apache.maven.plugins +maven-dependency-plugin + + + +generate-dependencies-raw Review comment: > I think it would make more sense to put the included artifact list and the assembly descriptor in a property set by the profile Easy enough This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services