> On Sept. 22, 2017, 2:20 a.m., András Piros wrote: > > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > > Lines 334 (patched) > > <https://reviews.apache.org/r/62470/diff/1/?file=1832290#file1832290line334> > > > > It's not very much readable ATM. Can you please call this very method > > another way like `setupLauncherAndDefaultChildEnv()`, inside call the old > > `setupLauncherConf()`, and have an empty implementation of > > `addDefaultChildEnv()` that can be overridden e.g. in `SparkActionExecutor`.
I have to create new setupLauncherConf() method to get actionConf. Not to disturb other action executor, I've overridden it here. actionConf has CONF_OOZIE_SPARK_SETUP_HADOOP_CONF_DIR which is needed while setting env variables. > On Sept. 22, 2017, 2:20 a.m., András Piros wrote: > > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java > > Lines 134-138 (patched) > > <https://reviews.apache.org/r/62470/diff/1/?file=1832291#file1832291line137> > > > > You really should have descriptive unit tests covering the changed / > > added functionality. Modified existing test case to include oozie.action.spark.setup.hadoop.conf.dir=true. So that we can see env variables in launcher conf. > On Sept. 22, 2017, 2:20 a.m., András Piros wrote: > > sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java > > Line 107 (original), 108 (patched) > > <https://reviews.apache.org/r/62470/diff/1/?file=1832292#file1832292line108> > > > > It seems to me that `_testSetupMethods()` is not gonna be called, so no > > use in extending that particular test method. Please either make it > > callable again, or extract to another test method that will be called. Why won't it get called? Its helper for testSetupMethods and testSetupMethodsWithSparkConfiguration - Satish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62470/#review185905 ----------------------------------------------------------- On Sept. 21, 2017, 9:54 a.m., Satish Saley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62470/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2017, 9:54 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-3062 > https://issues.apache.org/jira/browse/OOZIE-3062 > > > Repository: oozie-git > > > Description > ------- > > OOZIE-3062 Set HADOOP_CONF_DIR for spark action > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 9d1afb51 > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java > 80d64ec8 > > sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java > d97f1f06 > > > Diff: https://reviews.apache.org/r/62470/diff/1/ > > > Testing > ------- > > > Thanks, > > Satish Saley > >
