> On Oct. 23, 2012, 5:07 a.m., Santhosh Srinivasan wrote: > > A few comments. Its probably a good idea to have someone with more > > knowledge of HBaseStorage to take a second look.
Thank you very much for your feedback! I added answers below. Please let me know if you disagree with me. > On Oct. 23, 2012, 5:07 a.m., Santhosh Srinivasan wrote: > > ivy/libraries.properties, line 74 > > <https://reviews.apache.org/r/7676/diff/2/?file=178458#file178458line74> > > > > Zookeeper-3.4.4 has been out but has a known issue with SASL and Java > > 1.7. Is 3.3.3 required for Hbase 0.94.1 ? You're asking whether ZK 3.4.3 (not 3.3.3) is required by hbase 0.94.1, right? The answer is yes. In particular, HBaseTestingUtility depends on the following ZK class, which doesn't seem to exist in ZK 3.3.3: java.lang.NoClassDefFoundError: org/apache/zookeeper/server/NIOServerCnxnFactory In fact, I don't think that we should worry about those ZK known issues because the versions of HBase and ZK that I am updating only matter to unit test. As far as I can tell, HBaseStorage itself is fully compatible with all of HBase 0.90, 0.92, and 0.94 and won't be effected by this change at all. > On Oct. 23, 2012, 5:07 a.m., Santhosh Srinivasan wrote: > > test/org/apache/pig/test/TestJobSubmission.java, line 431 > > <https://reviews.apache.org/r/7676/diff/2/?file=178460#file178460line431> > > > > Can the commented out code be removed? To be honest, I do not know why we keep that block of code. Nevertheless, I am hesitating to remove it since someone might have commented it out only temporarily. - Cheolsoo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7676/#review12676 ----------------------------------------------------------- On Oct. 22, 2012, 6:50 a.m., Cheolsoo Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7676/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2012, 6:50 a.m.) > > > Review request for pig and Santhosh Srinivasan. > > > Description > ------- > > The changes include: > > 1. Stop bundling hbase.jar and zookeoper.jar with pig.jar. So there should be > no longer incompatibility issues when using pig.jar with different versions > of hbase.jar. But to use HBaseStorage, HBASE_HOME and ZOOKEEPER_HOME must be > set by the user. Note that I am adding protobuf-java.jar to pig.jar because > otherwise it has to be explicitly added to PIG_CLASSPATH to use HBaseStorage, > which is not very intuitive. > > 2. Bump hbase and zk to 0.94.1 and 3.4.3 respectively. Since we no longer > bundle them in pig.jar, which versions we use doesn't matter. These jar files > will be used for unit test only. > > 3. Make the unit test cases work with newer versions of hbase and zk. > > 4. Add hbase runtime dependencies to ivy.xml. > > > This addresses bug PIG-2885. > https://issues.apache.org/jira/browse/PIG-2885 > > > Diffs > ----- > > build.xml 6b04f8a > ivy.xml 6e0a2e5 > ivy/libraries.properties 55da6c6 > test/org/apache/pig/test/TestHBaseStorage.java cc1efef > test/org/apache/pig/test/TestJobSubmission.java 021662f > > Diff: https://reviews.apache.org/r/7676/diff/ > > > Testing > ------- > > ant clean test-commit -Dhadoopversion=20 > ant clean test -Dtestcase=TestHBaseStorage -Dhadoopversion=20 > ant clean test -Dtestcase=TestJobSumission -Dhadoopversion=20 > > I also manually tested pig.jar with hbase 0.90 and 0.94. Once HBASE_HOME and > ZOOKEEPER_HOME are set, HBaseStorage works fine with both versions. > > > Thanks, > > Cheolsoo Park > >
