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 <an...@cloudera.com> on 2017/05/30 08:50:48 UTC
Review Request 59635: OOZIE-2923 Improve Spark options parsing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/
-----------------------------------------------------------
Review request for oozie.
Repository: oozie-git
Description
-------
OOZIE-2923 Improve Spark options parsing
Diffs
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsBuilder.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
Diff: https://reviews.apache.org/r/59635/diff/1/
Testing
-------
Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
Thanks,
András Piros
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/#review176600
-----------------------------------------------------------
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java
Lines 137 (patched)
<https://reviews.apache.org/r/59635/#comment249971>
This code path is only active in yarn mode, please change the testDriverClassPathArgsParsing to test in yarn mode
- Peter Cseh
On May 31, 2017, 5:03 p.m., András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59635/
> -----------------------------------------------------------
>
> (Updated May 31, 2017, 5:03 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2923 Improve Spark options parsing
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/HadoopUriFinder.java PRE-CREATION
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/JarFilter.java PRE-CREATION
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java PRE-CREATION
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 2d4c83c6ecff4be306c2864d1fb9650f1b58dcf2
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java PRE-CREATION
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
>
>
> Diff: https://reviews.apache.org/r/59635/diff/3/
>
>
> Testing
> -------
>
> Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
>
>
> Thanks,
>
> András Piros
>
>
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/
-----------------------------------------------------------
(Updated June 7, 2017, 6:46 p.m.)
Review request for oozie.
Changes
-------
Addressing fixed order of parameters using `LinkedHashMap` for JDK7 and JDK8.
Repository: oozie-git
Description
-------
OOZIE-2923 Improve Spark options parsing
Diffs (updated)
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/HadoopUriFinder.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/JarFilter.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 2d4c83c6ecff4be306c2864d1fb9650f1b58dcf2
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
Diff: https://reviews.apache.org/r/59635/diff/5/
Changes: https://reviews.apache.org/r/59635/diff/4-5/
Testing
-------
Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
Thanks,
András Piros
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/
-----------------------------------------------------------
(Updated June 6, 2017, 8:59 a.m.)
Review request for oozie.
Repository: oozie-git
Description
-------
OOZIE-2923 Improve Spark options parsing
Diffs (updated)
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/HadoopUriFinder.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/JarFilter.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 2d4c83c6ecff4be306c2864d1fb9650f1b58dcf2
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
Diff: https://reviews.apache.org/r/59635/diff/4/
Changes: https://reviews.apache.org/r/59635/diff/3-4/
Testing
-------
Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
Thanks,
András Piros
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/
-----------------------------------------------------------
(Updated May 31, 2017, 5:03 p.m.)
Review request for oozie.
Changes
-------
Addressing the only `FINDBUGS_DIFF` issue.
Repository: oozie-git
Description
-------
OOZIE-2923 Improve Spark options parsing
Diffs (updated)
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/HadoopUriFinder.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/JarFilter.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 2d4c83c6ecff4be306c2864d1fb9650f1b58dcf2
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
Diff: https://reviews.apache.org/r/59635/diff/3/
Changes: https://reviews.apache.org/r/59635/diff/2-3/
Testing
-------
Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
Thanks,
András Piros
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/
-----------------------------------------------------------
(Updated May 31, 2017, 2:33 p.m.)
Review request for oozie.
Changes
-------
Addressing review comments.
Repository: oozie-git
Description
-------
OOZIE-2923 Improve Spark options parsing
Diffs (updated)
-----
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/HadoopUriFinder.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/JarFilter.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 2d4c83c6ecff4be306c2864d1fb9650f1b58dcf2
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java PRE-CREATION
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
Diff: https://reviews.apache.org/r/59635/diff/2/
Changes: https://reviews.apache.org/r/59635/diff/1-2/
Testing
-------
Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
Thanks,
András Piros
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/#review176305
-----------------------------------------------------------
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 48 (patched)
<https://reviews.apache.org/r/59635/#comment249702>
I don't see this option in any of the test cases, please add one.
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 202 (patched)
<https://reviews.apache.org/r/59635/#comment249703>
--conf screams for a constant :)
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 249 (patched)
<https://reviews.apache.org/r/59635/#comment249704>
There is a constant for this
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 257 (patched)
<https://reviews.apache.org/r/59635/#comment249705>
there is a constant for this
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java
Line 386 (original), 189 (patched)
<https://reviews.apache.org/r/59635/#comment249707>
I know it's unrelated, but please change "distcpConf" to "actionConf" or something similar. It's probably copy-paste code, but we'll consolidate it later
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java
Line 586 (original), 315 (patched)
<https://reviews.apache.org/r/59635/#comment249706>
If it does not cause big turmoil, please put this class to it's own file.
- Peter Cseh
On May 30, 2017, 8:50 a.m., András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59635/
> -----------------------------------------------------------
>
> (Updated May 30, 2017, 8:50 a.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2923 Improve Spark options parsing
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java PRE-CREATION
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsBuilder.java PRE-CREATION
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
>
>
> Diff: https://reviews.apache.org/r/59635/diff/1/
>
>
> Testing
> -------
>
> Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
>
>
> Thanks,
>
> András Piros
>
>
Re: Review Request 59635: OOZIE-2923 Improve Spark options parsing
Posted by Peter Bacsko <pb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59635/#review176307
-----------------------------------------------------------
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 35 (patched)
<https://reviews.apache.org/r/59635/#comment249708>
Is it legitimate to call this a builder?
I think it's better to rename it sth like SparkArgsUtil and build() could be called createFromMainArgs().
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 86-89 (patched)
<https://reviews.apache.org/r/59635/#comment249709>
All string literals could be constants, no?
sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java
Lines 232 (patched)
<https://reviews.apache.org/r/59635/#comment249710>
const
- Peter Bacsko
On máj. 30, 2017, 8:50 de, András Piros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59635/
> -----------------------------------------------------------
>
> (Updated máj. 30, 2017, 8:50 de)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2923 Improve Spark options parsing
>
>
> Diffs
> -----
>
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkArgsBuilder.java PRE-CREATION
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkMain.java 68f7a607344ac8b3c11ab08220f27139433a998e
> sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java PRE-CREATION
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsBuilder.java PRE-CREATION
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 5f6a0cdcde21287580f6b7037594669b508d24ef
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java 31f53acca24d1dd645e32a37b58643b7a0a3cacd
>
>
> Diff: https://reviews.apache.org/r/59635/diff/1/
>
>
> Testing
> -------
>
> Please see the JUnit tests `TestSparkMain`, `TestSparkOptionsSplitter`, and `TestSparkArgsBuilder` for test cases.
>
>
> Thanks,
>
> András Piros
>
>