> On March 4, 2017, 2:20 a.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/SparkConfigurationService.java
> > Lines 68 (patched)
> > <https://reviews.apache.org/r/57305/diff/1/?file=1655693#file1655693line73>
> >
> >     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?

ConfigurationService returns an empty array if the property is not set, not 
null. I'll add tests to check this.


> On March 4, 2017, 2:20 a.m., Robert Kanter wrote:
> > core/src/main/resources/oozie-default.xml
> > Lines 2896 (patched)
> > <https://reviews.apache.org/r/57305/diff/1/?file=1655694#file1655694line2896>
> >
> >     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.

If we set the default value spark.yarn.jar here, then it'll be added to the 
blacklist regardless of what 
oozie.service.SparkConfigurationService.spark.configurations.ignore.spark.yarn.jar
 is set to. I found that behavior confusing so decided to set empty string as 
default. I'll think about how treat this transition.


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review167920
-----------------------------------------------------------


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
> 
>

Reply via email to