----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3059/#review3722 -----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java <https://reviews.apache.org/r/3059/#comment8374> Using the same counter for stats (as for output) can result in cases where we can not infer just by looking at counter value - who appended it. It is better to have a separate counter for stats (in the same counter group). trunk/core/src/main/java/org/apache/oozie/action/hadoop/OoziePigStats.java <https://reviews.apache.org/r/3059/#comment8364> ActionType in ctor redundant trunk/core/src/main/java/org/apache/oozie/action/hadoop/OoziePigStats.java <https://reviews.apache.org/r/3059/#comment8363> [Styling comment] Setting separator = "" at start is confusing. An alternative can be something like: - String separator = ","; for (JobStats jobStats : jobGraph) { if (sb.length()) { sb.append(separator); } // do whatever you are doing } // .length() calls on containers is generally very efficient... trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java <https://reviews.apache.org/r/3059/#comment8365> Link to comment made on ctor signature earlier. trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java <https://reviews.apache.org/r/3059/#comment8366> Just return sb.toString() inside if block and you do not need the following else block and related indentation. trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java <https://reviews.apache.org/r/3059/#comment8367> Link to comment about separator = "" made earlier. - Prakhar On 2011-12-07 18:33:26, Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3059/ > ----------------------------------------------------------- > > (Updated 2011-12-07 18:33:26) > > > Review request for oozie, Mohammad Islam and Angelo K. Huang. > > > Summary > ------- > > This patch and https://reviews.apache.org/r/3047/diff addresses Oozie-588 > > > Docs for how to access pig stats through el functions needs to be added > > > This addresses bug Oozie-588. > https://issues.apache.org/jira/browse/Oozie-588 > > > Diffs > ----- > > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 1211573 > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java > 1211573 > trunk/core/src/main/java/org/apache/oozie/action/hadoop/OoziePigStats.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java > 1211573 > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java > 1211573 > trunk/core/src/main/resources/oozie-default.xml 1211573 > trunk/core/src/test/java/org/apache/oozie/action/hadoop/PigTestCase.java > 1211573 > > trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java > 1211573 > > trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java > 1211573 > trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPigMain.java > 1211573 > > Diff: https://reviews.apache.org/r/3059/diff > > > Testing > ------- > > Test cases for PigMain and PigAE added > Tested pig action after running examples > > > Thanks, > > Virag > >
