> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/conf/oozie-site.xml, line 25
> > <https://reviews.apache.org/r/2927/diff/1/?file=59983#file59983line25>
> >
> >     space

k


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java,
> >  line 139
> > <https://reviews.apache.org/r/2927/diff/1/?file=59985#file59985line139>
> >
> >     what is the value of using proto conf here?

with some versions of Hadoop kerberos name rules are not part of the default 
configuration, using the proto action conf ensures they always are there.


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java,
> >  line 135
> > <https://reviews.apache.org/r/2927/diff/1/?file=59986#file59986line135>
> >
> >     space

k


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 175
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line175>
> >
> >     Minor stuff. It might be better to use the variable "HIVE_SCRIPT" in 
> > place of hard-coding.

k


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 182
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line182>
> >
> >     the file name will be oozie-user-hive-default.xml?

On the HiveMain side is always this name, the HiveActionExecutor copies with 
this name.


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 211
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line211>
> >
> >     'dummy' file is needed to get the current directory. is it the only 
> > reason?

this is a common way of finding the current dir


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 241
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line241>
> >
> >     is it parameter 'key' or 'value' ? Sounds 'key' or 'var' more 
> > appropriate.

this is the hive action <param> thus it should be param


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 244
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line244>
> >
> >     is it possible 'val' could be empty? Any special checks needed? Or 
> > 'null' is valid value?

it can be empty but it cannot be null


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 316
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line316>
> >
> >     closing the BufferedReader might be safer in try{} finally{br.close()}

k


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java, 
> > line 323
> > <https://reviews.apache.org/r/2927/diff/1/?file=59987#file59987line323>
> >
> >     call it in try {}finall{os.close()} could be better

k


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/main/java/org/apache/oozie/service/WorkflowAppService.java, 
> > line 101
> > <https://reviews.apache.org/r/2927/diff/1/?file=59990#file59990line101>
> >
> >     what is the usage of 'conf' parameter?

this is odd, as LiteWorkflowAppService calls using the conf but is not in the 
diff.


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java,
> >  line 74
> > <https://reviews.apache.org/r/2927/diff/1/?file=59991#file59991line74>
> >
> >     is there any non-proprietary equivalent  stuff there?

No, this is hive land


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceMain.java,
> >  line 58
> > <https://reviews.apache.org/r/2927/diff/1/?file=59993#file59993line58>
> >
> >     if not intended, please take out this file.

k


> On 2011-11-29 10:06:50, Mohammad Islam wrote:
> > /trunk/core/src/test/java/org/apache/oozie/command/wf/TestSubmitCommand.java,
> >  line 60
> > <https://reviews.apache.org/r/2927/diff/1/?file=59994#file59994line60>
> >
> >     Wandering how come it was working?

don't know


- Alejandro


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


On 2011-11-23 21:59:10, Alejandro Abdelnur wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2927/
> -----------------------------------------------------------
> 
> (Updated 2011-11-23 21:59:10)
> 
> 
> Review request for oozie.
> 
> 
> Summary
> -------
> 
> OOZIE-68 Add Hive Action
> 
> 
> This addresses bug OOZIE-68.
>     https://issues.apache.org/jira/browse/OOZIE-68
> 
> 
> Diffs
> -----
> 
>   /trunk/.gitignore 1205620 
>   /trunk/client/src/main/java/org/apache/oozie/cli/OozieCLI.java 1205620 
>   /trunk/client/src/main/resources/hive-action-0.2.xsd PRE-CREATION 
>   /trunk/core/pom.xml 1205620 
>   /trunk/core/src/main/conf/oozie-site.xml 1205620 
>   /trunk/core/src/main/java/org/apache/oozie/ErrorCode.java 1205620 
>   
> /trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java
>  1205620 
>   
> /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java
>  PRE-CREATION 
>   /trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java 
> PRE-CREATION 
>   /trunk/core/src/main/java/org/apache/oozie/command/wf/SubmitCommand.java 
> 1205620 
>   
> /trunk/core/src/main/java/org/apache/oozie/service/LiteWorkflowAppService.java
>  1205620 
>   /trunk/core/src/main/java/org/apache/oozie/service/WorkflowAppService.java 
> 1205620 
>   
> /trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java
>  PRE-CREATION 
>   /trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestHiveMain.java 
> PRE-CREATION 
>   
> /trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceMain.java
>  1205620 
>   
> /trunk/core/src/test/java/org/apache/oozie/command/wf/TestSubmitCommand.java 
> 1205620 
>   
> /trunk/core/src/test/java/org/apache/oozie/service/TestLiteWorkflowAppService.java
>  1205620 
>   /trunk/core/src/test/resources/user-hive-default.xml PRE-CREATION 
>   /trunk/docs/src/site/twiki/DG_HiveActionExtension.twiki PRE-CREATION 
>   /trunk/docs/src/site/twiki/ENG_Building.twiki 1205620 
>   /trunk/docs/src/site/twiki/index.twiki 1205620 
>   /trunk/examples/pom.xml 1205620 
>   /trunk/examples/src/main/apps/hive/job.properties PRE-CREATION 
>   /trunk/examples/src/main/apps/hive/my-hive-default.xml PRE-CREATION 
>   /trunk/examples/src/main/apps/hive/script.q PRE-CREATION 
>   /trunk/examples/src/main/apps/hive/workflow.xml PRE-CREATION 
>   
> /trunk/examples/src/test/java/org/apache/oozie/example/TestLocalOozieExample.java
>  1205620 
>   /trunk/pom.xml 1205620 
>   /trunk/sharelib/hive/pom.xml PRE-CREATION 
>   /trunk/sharelib/pom.xml 1205620 
>   /trunk/src/main/assemblies/sharelib.xml 1205620 
> 
> Diff: https://reviews.apache.org/r/2927/diff
> 
> 
> Testing
> -------
> 
> Note that hive testcases are excluded by default and to run the you have to 
> do:
> 
>   mvn -DtestHive -Dtest=TestHiveActionExecutor,TestHiveMain
> 
> This is mentioned in the ENG_Building twiki.
> 
> The reason for this is the dependency conflict between pig and hive regarding 
> antlr
> 
> 
> Thanks,
> 
> Alejandro
> 
>

Reply via email to