> 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.
> 
> Alejandro Abdelnur wrote:
>     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.
> 
> Mohammad Islam wrote:
>     My use case is as follows:
>     1. <file>myscr.sh</file>
>     2. <exec>myscr.sh<exec>
>     
>     In this case, if Oozie LM executes the command, it will not find the 
> executable "myscr.sh" because PATH variable doesn't contain CWD(.).
>     However, if it is like <exec>./myscr.sh<exec>, it will work. That means 
> user has to give the full path of the executable. How can a user configure 
> this through PATH variable. 
>

well adding current dir to launcher process path PATH would do. just that.

but still i'd argue that unix users are use to do the ./SCRIPT thingy as 
current dir is commonly not in the PATH.

Else, we could in the ShellMain to look at the script name and if does not 
start with './' prepend it 


> 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. 
>
> 
> Alejandro Abdelnur wrote:
>     If using the ProcessBuilder class, you can easily set the workding 
> directory with the directory(File) method.
> 
> Mohammad Islam wrote:
>     Does it add the CWD in the classpath?

ProcessBuilder is to launch Shells, nothing specific to classpath handling


> 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?
>
> 
> Alejandro Abdelnur wrote:
>     Don't use Eclipse, use IntelliJ

sorry, I meant I DON'T USE ECLIPSE...


> 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?
> 
> Alejandro Abdelnur wrote:
>     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.
> 
> Mohammad Islam wrote:
>     Not sure about the capture-output. If user writes a lot of output for its 
> own debugging and we consider that as capture-output, it might not work or go 
> out-of-max-size error very often.
>     What we could do, by default, capture-output will be false.
>     If a user needs capture output, we should write it to the capture-output 
> file.
>

we should preserve the same functionality we have in SSH and Java action, if 
<capture-output/> is set, the STDOUT of the action is assumed to be in 
Properties file format and it becomes the action outputdata.

Any debugging the user wants to do it should go to STDERR. 

This should be clearly documented.


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

Reply via email to