You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Satish Saley <sa...@gmail.com> on 2017/10/03 19:35:27 UTC
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/
-----------------------------------------------------------
(Updated Oct. 3, 2017, 12:35 p.m.)
Review request for oozie.
Bugs: OOZIE-3062
https://issues.apache.org/jira/browse/OOZIE-3062
Repository: oozie-git
Description
-------
OOZIE-3062 Set HADOOP_CONF_DIR for spark action
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java be056038
core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
Diff: https://reviews.apache.org/r/62470/diff/3/
Changes: https://reviews.apache.org/r/62470/diff/2-3/
Testing
-------
Thanks,
Satish Saley
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/
-----------------------------------------------------------
(Updated Oct. 12, 2017, 1:09 p.m.)
Review request for oozie.
Bugs: OOZIE-3062
https://issues.apache.org/jira/browse/OOZIE-3062
Repository: oozie-git
Description
-------
OOZIE-3062 Set HADOOP_CONF_DIR for spark action
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 76d0daa4
core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java 9591cd95
core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
sharelib/spark/pom.xml b1fab104
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
Diff: https://reviews.apache.org/r/62470/diff/6/
Changes: https://reviews.apache.org/r/62470/diff/5-6/
Testing
-------
Thanks,
Satish Saley
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/
-----------------------------------------------------------
(Updated Oct. 6, 2017, 9:54 a.m.)
Review request for oozie.
Bugs: OOZIE-3062
https://issues.apache.org/jira/browse/OOZIE-3062
Repository: oozie-git
Description
-------
OOZIE-3062 Set HADOOP_CONF_DIR for spark action
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 76d0daa4
core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java 9591cd95
core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
sharelib/spark/pom.xml b1fab104
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
Diff: https://reviews.apache.org/r/62470/diff/5/
Changes: https://reviews.apache.org/r/62470/diff/4-5/
Testing
-------
Thanks,
Satish Saley
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/#review187072
-----------------------------------------------------------
Ship it!
Ship It!
- András Piros
On Oct. 3, 2017, 9:52 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62470/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2017, 9:52 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-3062
> https://issues.apache.org/jira/browse/OOZIE-3062
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3062 Set HADOOP_CONF_DIR for spark action
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 76d0daa4
> core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
>
>
> Diff: https://reviews.apache.org/r/62470/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/#review187014
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java
Lines 149-151 (original)
<https://reviews.apache.org/r/62470/#comment263966>
I missed this earlier in the review. If we have this, then we should set the env here (will require method signature change) and remove the earlier code for setting SPARK_HOME that was done in setupLauncherConf as well.
- Rohini Palaniswamy
On Oct. 3, 2017, 9:52 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62470/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2017, 9:52 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-3062
> https://issues.apache.org/jira/browse/OOZIE-3062
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3062 Set HADOOP_CONF_DIR for spark action
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 76d0daa4
> core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
>
>
> Diff: https://reviews.apache.org/r/62470/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/
-----------------------------------------------------------
(Updated Oct. 3, 2017, 2:52 p.m.)
Review request for oozie.
Bugs: OOZIE-3062
https://issues.apache.org/jira/browse/OOZIE-3062
Repository: oozie-git
Description
-------
OOZIE-3062 Set HADOOP_CONF_DIR for spark action
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 76d0daa4
core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
Diff: https://reviews.apache.org/r/62470/diff/4/
Changes: https://reviews.apache.org/r/62470/diff/3-4/
Testing
-------
Thanks,
Satish Saley
Re: Review Request 62470: OOZIE-3062 Set HADOOP_CONF_DIR for spark
action
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62470/#review186997
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
Line 331 (original), 331 (patched)
<https://reviews.apache.org/r/62470/#comment263948>
Would like to disagree with András Piros comment on creating yet another method in JavaActionExecutor. It does not make it clean. We should not be renaming method to include every step done inside the method. Adding default child env is still part of setting up the launcher config and it does not require special mention in the method name and a new method. I would suggest reverting back to old method name and overriding it as in the initial patch.
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java
Lines 136 (patched)
<https://reviews.apache.org/r/62470/#comment263949>
You can get rid of this and just use ArrayUtils.contains method from commons-lang
- Rohini Palaniswamy
On Oct. 3, 2017, 7:35 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62470/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2017, 7:35 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-3062
> https://issues.apache.org/jira/browse/OOZIE-3062
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-3062 Set HADOOP_CONF_DIR for spark action
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java be056038
> core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 80d64ec8
> sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java d97f1f06
>
>
> Diff: https://reviews.apache.org/r/62470/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Satish Saley
>
>