> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java,
> >  line 80
> > <https://reviews.apache.org/r/3059/diff/2/?file=63232#file63232line80>
> >
> >     private
> >     
> >     I do not understand why this needs to be a constant value?

The variable defines the default value of max stats.
It shouldn't be changed, so it is a constant.


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java,
> >  line 82
> > <https://reviews.apache.org/r/3059/diff/2/?file=63232#file63232line82>
> >
> >     private static final

Making a constant private wouldn't make sense. We would like reusability of 
this constant.


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java,
> >  line 979
> > <https://reviews.apache.org/r/3059/diff/2/?file=63232#file63232line979>
> >
> >     java doc format and parameters
> >     
> >     /**
> >     */

Will add but currently we don't have java doc for protected methods


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java,
> >  line 157
> > <https://reviews.apache.org/r/3059/diff/2/?file=63233#file63233line157>
> >
> >     java doc

ok


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/OoziePigStats.java, 
> > line 45
> > <https://reviews.apache.org/r/3059/diff/2/?file=63234#file63234line45>
> >
> >     move after java doc

ok


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java, line 
> > 309
> > <https://reviews.apache.org/r/3059/diff/2/?file=63236#file63236line309>
> >
> >     do you want to do this to keep stacktrace?
> >     
> >     throw new UnsupportedOperationException(
> >     307     
> >                                     "Pig stats are not supported for this 
> > type of operation", uoe);

very good catch..thanks


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigMain.java, line 
> > 341
> > <https://reviews.apache.org/r/3059/diff/2/?file=63236#file63236line341>
> >
> >     does it need to be static?

Yes. The method doesn't modify the state of object, so is static


> On 2011-12-09 02:31:54, Angelo K. Huang wrote:
> > trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java,
> >  line 367
> > <https://reviews.apache.org/r/3059/diff/2/?file=63240#file63240line367>
> >
> >     identation

ok


- Virag


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3059/#review3765
-----------------------------------------------------------


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
> 
>

Reply via email to