----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27614/#review62107 -----------------------------------------------------------
Looks good overall. I just had some minor, mostly trivial, comments this time. I'm sure you've done some testing. Have you made sure that a Spark job that fails is properly handled? core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java <https://reviews.apache.org/r/27614/#comment104050> Please use curly braces around the if statement. It's very easy for someone later to accidently mess something up here otherwise sharelib/spark/pom.xml <https://reviews.apache.org/r/27614/#comment104051> I know this is silly, but capitalize 's' in Spark sharelib/spark/pom.xml <https://reviews.apache.org/r/27614/#comment104052> In that case, can we add a property to the root pom so we can set the version there and reference it here? That way we're less likely to forget about this in the future. e.g. ${spark.guava.version} sharelib/spark/src/main/java/org.apache.oozie.action.hadoop/SparkMain.java <https://reviews.apache.org/r/27614/#comment104053> Can you add the below print statement here like we do in most of the other actions? e.g. System.out.println(); System.out.println("Oozie Spark action configuration"); System.out.println("================================================================="); System.out.println(); sharelib/spark/src/main/java/org.apache.oozie.action.hadoop/SparkMain.java <https://reviews.apache.org/r/27614/#comment104054> Can you add the below print statement here like we do in most of the other actions? System.out.println("================================================================="); System.out.println(); System.out.println(">>> Invoking Spark class now >>>"); System.out.println(); System.out.flush(); - Robert Kanter On Nov. 13, 2014, 6:49 a.m., pavan kumar kolamuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27614/ > ----------------------------------------------------------- > > (Updated Nov. 13, 2014, 6:49 a.m.) > > > Review request for oozie and shwethags. > > > Bugs: OOZIE-1983 > https://issues.apache.org/jira/browse/OOZIE-1983 > > > Repository: oozie-git > > > Description > ------- > > Add spark action executor in oozie. Spark jobs can be run using oozie > > > Diffs > ----- > > client/src/main/java/org/apache/oozie/cli/OozieCLI.java 9c2d14b > client/src/main/resources/spark-action-0.1.xsd PRE-CREATION > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java > PRE-CREATION > core/src/main/resources/oozie-default.xml 17155a1 > docs/src/site/twiki/DG_SparkActionExtension.twiki PRE-CREATION > docs/src/site/twiki/index.twiki c8ba742 > pom.xml 1e79186 > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/JavaMain.java > 8b8135a > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java > 8cfefbb > sharelib/pom.xml aa479a8 > sharelib/spark/pom.xml PRE-CREATION > sharelib/spark/src/main/java/org.apache.oozie.action.hadoop/SparkMain.java > PRE-CREATION > > sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java > PRE-CREATION > > sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java > PRE-CREATION > src/main/assemblies/sharelib.xml 4a46b90 > webapp/pom.xml 35776c5 > > Diff: https://reviews.apache.org/r/27614/diff/ > > > Testing > ------- > > Both unit testing and end to end testing done > > > Thanks, > > pavan kumar kolamuri > >
