incubator-oozie-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alejandro Abdelnur" <t...@cloudera.com>
Subject Re: Review Request: OOZIE-578 Support shell action in Oozie WF
Date Mon, 05 Dec 2011 20:50:26 GMT


> On 2011-12-05 18:40:43, Alejandro Abdelnur wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/client/src/main/resources/shell-action-0.1.xsd,
line 34
> > <https://reviews.apache.org/r/3000/diff/2/?file=61824#file61824line34>
> >
> >     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>
> >
> 
> Mohammad Islam wrote:
>     I considered that also an option. So user can specify it here and oozie will add
it to oozie.launcher.mapred.child.env.
>     I will add that.
>     In addition, currently, User could still add the env directly through oozie.launcher.mapred.child.env.

I don't think these ENV settings should affect the launcher ENV, these ENV settings are exclusively
for the SHELL it will be executed by the launcher.

If the user wants to alter the launcher ENV for some reason, they always can do it with the
oozie.launcher.mapred.child.env, but by definition, anything that is oozie.launcher.* is not
propagated to the action itself.


> On 2011-12-05 18:40:43, Alejandro Abdelnur wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java,
line 106
> > <https://reviews.apache.org/r/3000/diff/2/?file=61826#file61826line106>
> >
> >     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.
> 
> Mohammad Islam wrote:
>     I think we need it here. I spend a lot of time on this.
>     For example, my script is in CWD of LM. If I don't set it up in *.child.env. It will
never get the script if I do it RunTime.exec(args, envp).
>     Other environment passed in RunTime.exec(args, envp), doesn't have any impact on
finding the executables own path. However, it impacts on within the spawned script itself.
>     That's why I do it in *.child.env that will be available to LM to find the shell
executable itself as well as to the spawned script. 
>

If using the ProcessBuilder class, you can easily set the workding directory with the directory(File)
method.


> On 2011-12-05 18:40:43, Alejandro Abdelnur wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java,
line 29
> > <https://reviews.apache.org/r/3000/diff/2/?file=61827#file61827line29>
> >
> >     trailing spaces (through out the patch)
> 
> Mohammad Islam wrote:
>     I never add it. The oozie-foramtting.xml did it for me!! This is there for long time.
>     Do you have any new oozie-formatting xml?
>

Don't use Eclipse, use IntelliJ


> On 2011-12-05 18:40:43, Alejandro Abdelnur wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java,
line 74
> > <https://reviews.apache.org/r/3000/diff/2/?file=61827#file61827line74>
> >
> >     We should use ProcessBuilder instead Runtime.
> >     
> >     ProcessBuilder allows to easily set the environment.
> >     
> >
> 
> Mohammad Islam wrote:
>     I will check the ProcessBuilder. I thought both are same.

ProcessBuilder has some functionality that makes easier to deal with subprocesses, see prev
comment on setting directory.


> On 2011-12-05 18:40:43, Alejandro Abdelnur wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java,
line 132
> > <https://reviews.apache.org/r/3000/diff/2/?file=61827#file61827line132>
> >
> >     consuming the stdout/stderr of the forked process must be done in different
threads, else you may be be blocking processing/overflowing buffers.
> >     
> >
> 
> Mohammad Islam wrote:
>     Are you asking to spawn a new Thread to do it?

actually, 2 new threads, one to consume STDOUT and other STDERR, they should be daemon threads
and exit the loop on -1 read as well.

both outputs should be redirected to to the launcher process STDOUT/STDERR (as then they end
up in the launcher logs).

the STDOUT should also be captured (a la tee) so it can be used for <capture-output>
functionality.


> On 2011-12-05 18:40:43, Alejandro Abdelnur wrote:
> > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml,
line 1052
> > <https://reviews.apache.org/r/3000/diff/2/?file=61828#file61828line1052>
> >
> >     this should go in the wf.schemas property, no there.
> 
> Mohammad Islam wrote:
>     ok.

by bad, this is not there. We should open a JIRA for that.


- Alejandro


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


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message