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



http://svn.apache.org/repos/asf/incubator/oozie/trunk/client/src/main/resources/shell-action-0.1.xsd
<https://reviews.apache.org/r/3000/#comment8080>

    main-class is overridable via the oozie.launcher.main.class, no need for an 
element in the schema for it.



http://svn.apache.org/repos/asf/incubator/oozie/trunk/client/src/main/resources/shell-action-0.1.xsd
<https://reviews.apache.org/r/3000/#comment8083>

    env vars setting is missing, this is quite important when using shells.
    
    we should be able to specify multiple ENV vars, with one of the two 
syntaxes:
    
    <env-var>VAR=VALUE</env-var>
    
    or 
    
    <environment>
      <variable>
         <name>VALUE</name>
         <value>VALUE</value>
      </variable>
    </environment>
    



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
<https://reviews.apache.org/r/3000/#comment8084>

    missing license header



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
<https://reviews.apache.org/r/3000/#comment8085>

    remove automatically generated javadocs, they are noise. (this applies to 
all such javadocs in this patch)



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
<https://reviews.apache.org/r/3000/#comment8087>

    as the shellmain is forking a new process, these changes are not required 
for the launcher mapper process, but for the process the shellmain launches.



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
<https://reviews.apache.org/r/3000/#comment8086>

    trailing spaces (through out the patch)



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
<https://reviews.apache.org/r/3000/#comment8088>

    We should use ProcessBuilder instead Runtime.
    
    ProcessBuilder allows to easily set the environment.
    
    



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
<https://reviews.apache.org/r/3000/#comment8089>

    consuming the stdout/stderr of the forked process must be done in different 
threads, else you may be be blocking processing/overflowing buffers.
    
    



http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml
<https://reviews.apache.org/r/3000/#comment8090>

    this should go in the wf.schemas property, no there.


- Alejandro


On 2011-12-04 08:05:24, Mohammad Islam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3000/
> -----------------------------------------------------------
> 
> (Updated 2011-12-04 08:05:24)
> 
> 
> Review request for oozie.
> 
> 
> Summary
> -------
> 
> More context at JIRA OOZIE-578.
> 
> 
> This addresses bug OOZIE-578.
>     https://issues.apache.org/jira/browse/OOZIE-578
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/client/src/main/java/org/apache/oozie/cli/OozieCLI.java
>  1209346 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/client/src/main/resources/shell-action-0.1.xsd
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
>  1209346 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml
>  1209346 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/ShellTestCase.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellMain.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/DG_ShellActionExtension.twiki
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/index.twiki
>  1209346 
> 
> Diff: https://reviews.apache.org/r/3000/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mohammad
> 
>

Reply via email to