You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Peter Cseh <ge...@cloudera.com> on 2017/03/03 21:45:16 UTC
Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/
-----------------------------------------------------------
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
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
> On March 4, 2017, 7:06 a.m., Peter Cseh wrote:
> > core/src/main/resources/oozie-default.xml
> > Lines 2884-2891 (original), 2884-2892 (patched)
> > <https://reviews.apache.org/r/57305/diff/1/?file=1655694#file1655694line2884>
> >
> > Should I remove this property instead of adding this comment? We can still check it's value in SparkConfigurationService.
I would leave it. I think it will be confusing to users if we remove it from here, and it's suddenly an undocumented property.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review167932
-----------------------------------------------------------
On March 5, 2017, 9:53 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 5, 2017, 9:53 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/3/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review167932
-----------------------------------------------------------
core/src/main/resources/oozie-default.xml
Lines 2884-2891 (original), 2884-2892 (patched)
<https://reviews.apache.org/r/57305/#comment239923>
Should I remove this property instead of adding this comment? We can still check it's value in SparkConfigurationService.
- Peter Cseh
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
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
> On March 6, 2017, 9:30 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java
> > Line 35 (original)
> > <https://reviews.apache.org/r/57305/diff/1-3/?file=1655692#file1655692line35>
> >
> > Unrelated change.
Sorry. Ignore this. Reviewboard tricked me.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review168043
-----------------------------------------------------------
On March 5, 2017, 9:53 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 5, 2017, 9:53 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/3/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
> On March 6, 2017, 9:30 p.m., Robert Kanter wrote:
> >
There also seems to be a ton of findbug errors on the JIRA
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review168043
-----------------------------------------------------------
On March 5, 2017, 9:53 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 5, 2017, 9:53 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/3/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review168043
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java
Line 35 (original)
<https://reviews.apache.org/r/57305/#comment240068>
Unrelated change.
core/src/main/java/org/apache/oozie/service/SparkConfigurationService.java
Line 68 (original), 68-70 (patched)
<https://reviews.apache.org/r/57305/#comment240072>
This is a perfect example of where Java 8 lambda expressions would be helpful. Oh well
- Robert Kanter
On March 5, 2017, 9:53 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 5, 2017, 9:53 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/3/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review168048
-----------------------------------------------------------
core/src/main/resources/oozie-default.xml
Line 298 (original), 298 (patched)
<https://reviews.apache.org/r/57305/#comment240076>
The extra whitespace changes are still here.
core/src/main/resources/oozie-default.xml
Line 2884 (original), 2884 (patched)
<https://reviews.apache.org/r/57305/#comment240078>
I don't think we should remove this. That makes it an undocumented config property. We should just note that it is deprecated and mention the new property.
core/src/test/java/org/apache/oozie/service/TestSparkConfigurationService.java
Lines 95-97 (original), 59-61 (patched)
<https://reviews.apache.org/r/57305/#comment240079>
I think this is easier to read with the original indenting.
core/src/test/java/org/apache/oozie/service/TestSparkConfigurationService.java
Line 128 (original), 93 (patched)
<https://reviews.apache.org/r/57305/#comment240080>
I think this is easier to read with the original indenting.
core/src/test/java/org/apache/oozie/service/TestSparkConfigurationService.java
Lines 136-138 (patched)
<https://reviews.apache.org/r/57305/#comment240081>
I like the other indenting better.
- Robert Kanter
On March 5, 2017, 9:53 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 5, 2017, 9:53 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/3/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review168189
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Kanter
On March 7, 2017, 8:36 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 7, 2017, 8:36 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/5/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/
-----------------------------------------------------------
(Updated March 7, 2017, 8:36 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 (updated)
-----
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/5/
Changes: https://reviews.apache.org/r/57305/diff/4-5/
Testing
-------
Existing tests passed and added some more.
Thanks,
Peter Cseh
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/
-----------------------------------------------------------
(Updated March 7, 2017, 8:39 a.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 (updated)
-----
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/4/
Changes: https://reviews.apache.org/r/57305/diff/3-4/
Testing
-------
Existing tests passed and added some more.
Thanks,
Peter Cseh
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/
-----------------------------------------------------------
(Updated March 5, 2017, 9:53 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 (updated)
-----
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/3/
Changes: https://reviews.apache.org/r/57305/diff/2-3/
Testing
-------
Existing tests passed and added some more.
Thanks,
Peter Cseh
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/
-----------------------------------------------------------
(Updated March 5, 2017, 9:46 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 (updated)
-----
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/2/
Changes: https://reviews.apache.org/r/57305/diff/1-2/
Testing
-------
Existing tests passed and added some more.
Thanks,
Peter Cseh
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
> On March 3, 2017, 9:47 p.m., Peter Cseh wrote:
> > core/src/main/resources/oozie-default.xml
> > Line 293 (original), 293 (patched)
> > <https://reviews.apache.org/r/57305/diff/1/?file=1655694#file1655694line293>
> >
> > It looks like IDEA did something funny here. I'll remove the whitespaces.
Turn out it had some trailing whitespaces. I'm removing those
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review167871
-----------------------------------------------------------
On March 5, 2017, 9:46 p.m., Peter Cseh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57305/
> -----------------------------------------------------------
>
> (Updated March 5, 2017, 9:46 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/2/
>
>
> Testing
> -------
>
> Existing tests passed and added some more.
>
>
> Thanks,
>
> Peter Cseh
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57305/#review167871
-----------------------------------------------------------
core/src/main/resources/oozie-default.xml
Line 293 (original), 293 (patched)
<https://reviews.apache.org/r/57305/#comment239834>
It looks like IDEA did something funny here. I'll remove the whitespaces.
- Peter Cseh
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
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
> 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
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Peter Cseh <ge...@cloudera.com>.
- 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
>
>
Re: Review Request 57305: OOZIE-2811 Add support for filtering out
properties from SparkConfigurationService
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
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
>
>