You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by András Piros via Review Board <no...@reviews.apache.org> on 2018/05/14 11:04:05 UTC
Review Request 67112: OOZIE-3228 [Spark action] Can't load properties
from spark-defaults.conf
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/
-----------------------------------------------------------
Review request for oozie and Peter Cseh.
Repository: oozie-git
Description
-------
OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
Diffs
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 2f3cfbe4dfa2c2015686d2f2ca09ec4de544f423
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
Diff: https://reviews.apache.org/r/67112/diff/1/
Testing
-------
`mvn clean test -Dtest=TestSparkArgsExtractor`
Thanks,
András Piros
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/
-----------------------------------------------------------
(Updated May 16, 2018, 11:43 a.m.)
Review request for oozie and Peter Cseh.
Repository: oozie-git
Description
-------
OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
Diffs (updated)
-----
docs/src/site/twiki/DG_SparkActionExtension.twiki 66076ff9d3098aeb06301458b900da8d7f5fb5db
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 5af39cda52b403d8989fdd5d4f2524395a55580a
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
Diff: https://reviews.apache.org/r/67112/diff/4/
Changes: https://reviews.apache.org/r/67112/diff/3-4/
Testing
-------
`mvn clean test -Dtest=TestSparkArgsExtractor`
Thanks,
András Piros
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On May 16, 2018, 9:57 a.m., Peter Cseh wrote:
> > sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
> > Lines 379-398 (patched)
> > <https://reviews.apache.org/r/67112/diff/3/?file=2023950#file2023950line379>
> >
> > Can you also add this on the twiki level? Most users don't check the source code if something happens they don't expect.
Good idea! Added to `DG_SparkActionExtension.twiki`.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/#review203207
-----------------------------------------------------------
On May 16, 2018, 9:23 a.m., András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67112/
> -----------------------------------------------------------
>
> (Updated May 16, 2018, 9:23 a.m.)
>
>
> Review request for oozie and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 5af39cda52b403d8989fdd5d4f2524395a55580a
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
>
>
> Diff: https://reviews.apache.org/r/67112/diff/3/
>
>
> Testing
> -------
>
> `mvn clean test -Dtest=TestSparkArgsExtractor`
>
>
> Thanks,
>
> András Piros
>
>
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by Peter Cseh via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/#review203207
-----------------------------------------------------------
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
Lines 379-398 (patched)
<https://reviews.apache.org/r/67112/#comment285301>
Can you also add this on the twiki level? Most users don't check the source code if something happens they don't expect.
- Peter Cseh
On May 16, 2018, 9:23 a.m., András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67112/
> -----------------------------------------------------------
>
> (Updated May 16, 2018, 9:23 a.m.)
>
>
> Review request for oozie and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 5af39cda52b403d8989fdd5d4f2524395a55580a
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
>
>
> Diff: https://reviews.apache.org/r/67112/diff/3/
>
>
> Testing
> -------
>
> `mvn clean test -Dtest=TestSparkArgsExtractor`
>
>
> Thanks,
>
> András Piros
>
>
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/
-----------------------------------------------------------
(Updated May 16, 2018, 9:23 a.m.)
Review request for oozie and Peter Cseh.
Repository: oozie-git
Description
-------
OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
Diffs (updated)
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 5af39cda52b403d8989fdd5d4f2524395a55580a
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
Diff: https://reviews.apache.org/r/67112/diff/3/
Changes: https://reviews.apache.org/r/67112/diff/2-3/
Testing
-------
`mvn clean test -Dtest=TestSparkArgsExtractor`
Thanks,
András Piros
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/
-----------------------------------------------------------
(Updated May 15, 2018, 1:52 p.m.)
Review request for oozie and Peter Cseh.
Repository: oozie-git
Description
-------
OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
Diffs (updated)
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 5af39cda52b403d8989fdd5d4f2524395a55580a
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
Diff: https://reviews.apache.org/r/67112/diff/2/
Changes: https://reviews.apache.org/r/67112/diff/1-2/
Testing
-------
`mvn clean test -Dtest=TestSparkArgsExtractor`
Thanks,
András Piros
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On May 14, 2018, 11:23 a.m., Peter Cseh wrote:
> > sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
> > Lines 381-389 (patched)
> > <https://reviews.apache.org/r/67112/diff/1/?file=2021199#file2021199line386>
> >
> > This part feels similar to parts of https://github.com/apache/oozie/blob/master/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java
> >
> > Is it possible to reuse code from there?
> > It's possible I'm missing something that makes that not feasable.
`LauncherConfigurationInjector.java` works on the launcher `Configuration` level, while this code works on `Properties source, List<String> target` level. The semantics is similar, but not the same - means are different.
I'd keep them separate, as responsibility is also different.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/#review203009
-----------------------------------------------------------
On May 14, 2018, 11:04 a.m., András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67112/
> -----------------------------------------------------------
>
> (Updated May 14, 2018, 11:04 a.m.)
>
>
> Review request for oozie and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 2f3cfbe4dfa2c2015686d2f2ca09ec4de544f423
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
>
>
> Diff: https://reviews.apache.org/r/67112/diff/1/
>
>
> Testing
> -------
>
> `mvn clean test -Dtest=TestSparkArgsExtractor`
>
>
> Thanks,
>
> András Piros
>
>
Re: Review Request 67112: OOZIE-3228 [Spark action] Can't load
properties from spark-defaults.conf
Posted by Peter Cseh via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67112/#review203009
-----------------------------------------------------------
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
Line 41 (original), 41 (patched)
<https://reviews.apache.org/r/67112/#comment285056>
Please don't use *
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
Lines 85 (patched)
<https://reviews.apache.org/r/67112/#comment285058>
This one is used only in tests if I see correctly. Please move it ther.
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
Lines 375-378 (patched)
<https://reviews.apache.org/r/67112/#comment285059>
Please add documentation about the affected properties and some reasoning on why are we doing this.
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
Lines 381-389 (patched)
<https://reviews.apache.org/r/67112/#comment285057>
This part feels similar to parts of https://github.com/apache/oozie/blob/master/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java
Is it possible to reuse code from there?
It's possible I'm missing something that makes that not feasable.
- Peter Cseh
On May 14, 2018, 11:04 a.m., András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67112/
> -----------------------------------------------------------
>
> (Updated May 14, 2018, 11:04 a.m.)
>
>
> Review request for oozie and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3228 [Spark action] Can't load properties from spark-defaults.conf
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java 2f3cfbe4dfa2c2015686d2f2ca09ec4de544f423
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java 6a9baa5c7997198ab69f039f6a8884e49b13cad3
>
>
> Diff: https://reviews.apache.org/r/67112/diff/1/
>
>
> Testing
> -------
>
> `mvn clean test -Dtest=TestSparkArgsExtractor`
>
>
> Thanks,
>
> András Piros
>
>