----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3000/#review3703 -----------------------------------------------------------
http://svn.apache.org/repos/asf/incubator/oozie/trunk/client/src/main/resources/shell-action-0.1.xsd <https://reviews.apache.org/r/3000/#comment8284> we shouldn't have java-opts in the schema. users can always use the oozie.laucher.hadoop.child.... property if they need to tweak something for the launcher. 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/#comment8285> there are trailing space 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/#comment8271> here the files in the current dir should be printed to STDOUT. 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/#comment8272> remove commented code 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/#comment8273> this println is not needed, 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/#comment8274> remove this println 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/#comment8275> remove this println 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/#comment8277> STDOUT and STDERR streams should be closed in the thread run() method, after the while read exits. Not here as the Thread.start() is not blocking 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/#comment8278> here there should the finally block closing the streams http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/DG_ShellActionExtension.twiki <https://reviews.apache.org/r/3000/#comment8280> It would be more appropriate to state that Shell launcher configuration can be specified ...., correct? Currently the configuration (actionconf.xml) file is not made avail to the shell. Which makes me think that we should expose the the actionconf.xml file to the shell by setting a OOZIE_ACTION_CONF env var pointing to the file (it the ShellMain we would set that ENV to the value of the system property that has the path) http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/DG_ShellActionExtension.twiki <https://reviews.apache.org/r/3000/#comment8282> capture-output is not in the syntax, nor explained http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/DG_ShellActionExtension.twiki <https://reviews.apache.org/r/3000/#comment8281> while this example is 100% correct, I would use instead an example that is calling a shell instead of calling Java. For Java user should use the Java action. Another thing to mention is that when setting ENV variables, if a user want sto reference other ENV variables, he/she should use $VAR (never use ${VAR} as that would be processed as an EL) http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/DG_ShellActionExtension.twiki <https://reviews.apache.org/r/3000/#comment8283> we shouldn't have java-opts in the schema. users can always use the oozie.laucher.hadoop.child.... property if they need to tweak something for the launcher. - Alejandro On 2011-12-07 08:27:05, Mohammad Islam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3000/ > ----------------------------------------------------------- > > (Updated 2011-12-07 08:27:05) > > > 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/docs/src/site/twiki/index.twiki > 1209346 > > 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/core/src/test/java/org/apache/oozie/action/hadoop/TestShellMain.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/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/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/conf/oozie-site.xml > 1209346 > > 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 > > Diff: https://reviews.apache.org/r/3000/diff > > > Testing > ------- > > > Thanks, > > Mohammad > >
