> 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 33 > > <https://reviews.apache.org/r/3000/diff/2/?file=61824#file61824line33> > > > > main-class is overridable via the oozie.launcher.main.class, no need > > for an element in the schema for it.
ok > 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> > > 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. > 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 1 > > <https://reviews.apache.org/r/3000/diff/2/?file=61826#file61826line1> > > > > missing license header ok. > 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 22 > > <https://reviews.apache.org/r/3000/diff/2/?file=61826#file61826line22> > > > > remove automatically generated javadocs, they are noise. (this applies > > to all such javadocs in this patch) ok. > 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. 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. > 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) 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? > 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. > > > > I will check the ProcessBuilder. I thought both are same. > 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. > > > > Are you asking to spawn a new Thread to do it? > 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. ok. - Mohammad ----------------------------------------------------------- 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 > >
