----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57305/#review167920 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/service/SparkConfigurationService.java Lines 68 (patched) <https://reviews.apache.org/r/57305/#comment239901> Should we do a ````trim()```` on these? Also, what happens if ````oozie.service.SparkConfigurationService.spark.configurations.blacklist```` is set to empty string vs whitespace in oozie-site? Configuration treats the former as ````null```` and the latter as empty string (I know that's confusing). Does ````Arrays.asList```` handle these cases? core/src/main/java/org/apache/oozie/service/SparkConfigurationService.java Lines 72-73 (patched) <https://reviews.apache.org/r/57305/#comment239900> Can use the constants here instead of the strings. core/src/main/java/org/apache/oozie/service/SparkConfigurationService.java Line 71 (original), 87 (patched) <https://reviews.apache.org/r/57305/#comment239902> What about ````trim()````? core/src/main/resources/oozie-default.xml Lines 2896 (patched) <https://reviews.apache.org/r/57305/#comment239917> Do you think we should set some defaults in here? For instance, ````spark.yarn.jar```` and ````spark.yarn.jars````? You're more familiar with which properties Spark doesn't like, so I'll leave this up to you; but I think it's good to include things here that we'd always want to remove. core/src/test/java/org/apache/oozie/service/TestSparkConfigurationService.java Lines 144-154 (patched) <https://reviews.apache.org/r/57305/#comment239916> missing space after comma - Robert Kanter On March 3, 2017, 9:45 p.m., Peter Cseh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57305/ > ----------------------------------------------------------- > > (Updated March 3, 2017, 9:45 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-2811 > https://issues.apache.org/jira/browse/OOZIE-2811 > > > Repository: oozie-git > > > Description > ------- > > OOZIE-2811 > Add support for filtering out properties from SparkConfigurationService > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java > e2e023e6d9a14fa1feb8abb2028dfbd67a163090 > core/src/main/java/org/apache/oozie/service/SparkConfigurationService.java > b29ab8d80b7d900603b4697e54a03365fc2e8634 > core/src/main/resources/oozie-default.xml > 95e0c36e1b2f5aef18637f6f956a990ba36166f2 > > core/src/test/java/org/apache/oozie/service/TestSparkConfigurationService.java > 9d82fdc1a75be5c17da36f1cc2189246e2be0859 > > > Diff: https://reviews.apache.org/r/57305/diff/1/ > > > Testing > ------- > > Existing tests passed and added some more. > > > Thanks, > > Peter Cseh > >
