> On 2011-12-08 01:31:02, Prakhar Sharma wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, > > line 473 > > <https://reviews.apache.org/r/3059/diff/1/?file=62926#file62926line473> > > > > 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).
good point, separate counter added > On 2011-12-08 01:31:02, Prakhar Sharma wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/OoziePigStats.java, > > line 40 > > <https://reviews.apache.org/r/3059/diff/1/?file=62927#file62927line40> > > > > ActionType in ctor redundant changed > On 2011-12-08 01:31:02, Prakhar Sharma wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java, line > > 368 > > <https://reviews.apache.org/r/3059/diff/1/?file=62929#file62929line368> > > > > Just return sb.toString() inside if block and you do not need the > > following else block and related indentation. done > On 2011-12-08 01:31:02, Prakhar Sharma wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/OoziePigStats.java, > > line 73 > > <https://reviews.apache.org/r/3059/diff/1/?file=62927#file62927line73> > > > > [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... done - Virag ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3059/#review3722 ----------------------------------------------------------- On 2011-12-08 08:54:20, Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3059/ > ----------------------------------------------------------- > > (Updated 2011-12-08 08:54:20) > > > 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/test/java/org/apache/oozie/action/hadoop/TestPigMain.java > 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/main/resources/oozie-default.xml 1211573 > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java > 1211573 > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.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/JavaActionExecutor.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 > >
