----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25066/#review51607 -----------------------------------------------------------
Looks good overall, other than some minor things. Also, please make sure to run unit tests (and if possible, e2e testing) with Hadoop 1.x, 2.x, and 0.23.x and both DistCp v1 and v2 where applicable. core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java <https://reviews.apache.org/r/25066/#comment90053> @Override sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java <https://reviews.apache.org/r/25066/#comment90054> I think you can get rid of the "throwing exception" part of the message, that's pretty obvious :) sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java <https://reviews.apache.org/r/25066/#comment90055> Shouldn't this be break? Otherwise, it will just go to the next constructor in the loop. sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java <https://reviews.apache.org/r/25066/#comment90059> Shouldn't we be setting constArgs[1] to the DistCpOptions? Even if we're passing null, it would be best to explicetly set it (i.e. constArgs[1] = null;) sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java <https://reviews.apache.org/r/25066/#comment90056> Same as the other; it should be break. - Robert Kanter On Aug. 26, 2014, 5:11 p.m., Ryota Egashira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25066/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2014, 5:11 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1728 > https://issues.apache.org/jira/browse/OOZIE-1728 > > > Repository: oozie-git > > > Description > ------- > > https://issues.apache.org/jira/browse/OOZIE-1728 > > > Diffs > ----- > > core/pom.xml 5b2eedc > core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java > fe31d7b > > core/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java > PRE-CREATION > core/src/test/java/org/apache/oozie/action/hadoop/TestDistcpMain.java > PRE-CREATION > sharelib/distcp/pom.xml 04e436d > > sharelib/distcp/src/main/java/org/apache/oozie/action/hadoop/DistcpMain.java > PRE-CREATION > > sharelib/distcp/src/test/java/org/apache/oozie/action/hadoop/TestDistCpActionExecutor.java > b075957 > > Diff: https://reviews.apache.org/r/25066/diff/ > > > Testing > ------- > > upload patch for review, doing e2e test now. > > > Thanks, > > Ryota Egashira > >
