> On Nov. 19, 2014, 2:01 a.m., Robert Kanter wrote: > > 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?
Yes i have tested end to end , yes it was handled when spark job fails even the workflow will fail. > On Nov. 19, 2014, 2:01 a.m., Robert Kanter wrote: > > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java, > > lines 85-86 > > <https://reviews.apache.org/r/27614/diff/3/?file=761959#file761959line85> > > > > Please use curly braces around the if statement. It's very easy for > > someone later to accidently mess something up here otherwise Ok > On Nov. 19, 2014, 2:01 a.m., Robert Kanter wrote: > > sharelib/spark/pom.xml, lines 31-32 > > <https://reviews.apache.org/r/27614/diff/3/?file=761967#file761967line31> > > > > I know this is silly, but capitalize 's' in Spark Done > On Nov. 19, 2014, 2:01 a.m., Robert Kanter wrote: > > sharelib/spark/pom.xml, line 45 > > <https://reviews.apache.org/r/27614/diff/3/?file=761967#file761967line45> > > > > 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} Sure will use this > On Nov. 19, 2014, 2:01 a.m., Robert Kanter wrote: > > sharelib/spark/src/main/java/org.apache.oozie.action.hadoop/SparkMain.java, > > line 45 > > <https://reviews.apache.org/r/27614/diff/3/?file=761968#file761968line45> > > > > 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(); will add > On Nov. 19, 2014, 2:01 a.m., Robert Kanter wrote: > > sharelib/spark/src/main/java/org.apache.oozie.action.hadoop/SparkMain.java, > > line 93 > > <https://reviews.apache.org/r/27614/diff/3/?file=761968#file761968line93> > > > > 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(); will add - pavan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27614/#review62107 ----------------------------------------------------------- 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 > >