stoty commented on code in PR #6184:
URL: https://github.com/apache/hbase/pull/6184#discussion_r1804146366
##########
hbase-assembly/src/main/assembly/client.xml:
##########
@@ -45,6 +45,8 @@
<dependencySets>
<dependencySet>
<excludes>
+ <exclude>org.apache.hadoop:*:test-jar</exclude>
Review Comment:
Are those included without this exclusion, or is this just a safety measure ?
##########
bin/hbase:
##########
@@ -766,13 +777,17 @@ elif [ "$COMMAND" = "copyreppeers" ] ; then
CLASS='org.apache.hadoop.hbase.replication.CopyReplicationPeers'
else
CLASS=$COMMAND
-if [[ "$CLASS" =~ .*IntegrationTest.* ]] ; then
+
+ if [[ "$CLASS" =~ .*IntegrationTest.* ]] || [[ "$CLASS" =~ .*Chaos.* ]]; then
for f in ${HBASE_HOME}/lib/test/*.jar; do
if [ -f "${f}" ]; then
CLASSPATH="${CLASSPATH}:${f}"
fi
done
+
+ add_jars_from_compiled_source
Review Comment:
Why is this needed ?
Aren't we adding the non-test jars already (before this patch) ?
##########
hbase-assembly/src/main/assembly/hadoop-three-compat.xml:
##########
@@ -33,7 +33,6 @@
<useAllReactorProjects>true</useAllReactorProjects>
<includes>
<!-- Keep this list sorted by name -->
- <include>org.apache.hbase:hbase-annotations</include>
Review Comment:
Interesting. I didn't know these are only used for tests.
##########
hbase-it/pom.xml:
##########
@@ -375,6 +375,7 @@
<dependency>
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-minicluster</artifactId>
+ <scope>test</scope>
Review Comment:
I think we should leve these as compile scope.
hbase-it is dependend upon by several external projects, and having these in
compile scope ensures that they will get the transitive dependencies without
having to depend on them explicitly.
Now I'm not sure that depending on hbase-it is actually a good idea,
external projects should probably depend on hbase-testing-util, and any other
necessary hbase module explicitly, but even Phoenix depends on hbase-it.
##########
bin/hbase:
##########
@@ -766,13 +777,17 @@ elif [ "$COMMAND" = "copyreppeers" ] ; then
CLASS='org.apache.hadoop.hbase.replication.CopyReplicationPeers'
else
CLASS=$COMMAND
-if [[ "$CLASS" =~ .*IntegrationTest.* ]] ; then
+
+ if [[ "$CLASS" =~ .*IntegrationTest.* ]] || [[ "$CLASS" =~ .*Chaos.* ]]; then
for f in ${HBASE_HOME}/lib/test/*.jar; do
if [ -f "${f}" ]; then
CLASSPATH="${CLASSPATH}:${f}"
fi
done
+
+ add_jars_from_compiled_source
Review Comment:
If this is for Junit, etc, then IMO we should still copy them to lib/test
(just skip them when packaging the assemblies)
##########
hbase-thrift/pom.xml:
##########
@@ -444,6 +444,7 @@
<dependency>
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-minicluster</artifactId>
+ <scope>test</scope>
Review Comment:
I think it's fine to keep this in one patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
