You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Jason Phelps via Review Board <no...@reviews.apache.org> on 2018/02/13 15:07:23 UTC
Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65636/
-----------------------------------------------------------
Review request for oozie.
Repository: oozie-git
Description
-------
Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
Diffs
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java e581e50
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java a4d3e2f
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java e71a662
Diff: https://reviews.apache.org/r/65636/diff/1/
Testing
-------
Thanks,
Jason Phelps
Re: Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65636/#review214235
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java
Line 138 (original), 138 (patched)
<https://reviews.apache.org/r/65636/#comment300425>
We could use the diamond operator here.
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java
Lines 146 (patched)
<https://reviews.apache.org/r/65636/#comment300430>
foreach would be nicer
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java
Lines 162-164 (patched)
<https://reviews.apache.org/r/65636/#comment300426>
for (Path configDefaultFile : configDefaults) would be nicer.
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java
Lines 168 (patched)
<https://reviews.apache.org/r/65636/#comment300427>
Why is it System.out instead of LOG?
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java
Line 156 (original), 177 (patched)
<https://reviews.apache.org/r/65636/#comment300428>
I think it would be useful to add configDefaultFile to the error message.
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java
Line 159 (original), 180 (patched)
<https://reviews.apache.org/r/65636/#comment300431>
As far as I understand in the following scenario:
i=0 and fs.exists(configDefaultFile) == true: we create a new defaultConfigs
i=1 and fs.exists(configDefaultFile) == false: we skip the if in line 167 and we don't create a new defaultConfigs.
In this case the if in line 180 will refer to the old defaultConfigs and call copy again using the old data.
Could you please check this scenario and fix it.
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java
Lines 506-516 (patched)
<https://reviews.apache.org/r/65636/#comment300429>
Please add assert messages to the assertEquals.
- Andras Salamon
On March 29, 2019, 1 p.m., Jason Phelps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65636/
> -----------------------------------------------------------
>
> (Updated March 29, 2019, 1 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java 2862d33f
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 70b9adc1
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 2bc0baa4
>
>
> Diff: https://reviews.apache.org/r/65636/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Phelps
>
>
Re: Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65636/#review217883
-----------------------------------------------------------
Ship it!
Ship It!
- Andras Salamon
On Aug. 30, 2019, 8:53 p.m., Jason Phelps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65636/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2019, 8:53 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java 1073cb924
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 1352db91b
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 2cd263f60
>
>
> Diff: https://reviews.apache.org/r/65636/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Phelps
>
>
Re: Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
Posted by Jason Phelps via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65636/
-----------------------------------------------------------
(Updated Aug. 30, 2019, 8:53 p.m.)
Review request for oozie.
Changes
-------
Implemented requested changes
Repository: oozie-git
Description
-------
Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java 1073cb924
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 1352db91b
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 2cd263f60
Diff: https://reviews.apache.org/r/65636/diff/4/
Changes: https://reviews.apache.org/r/65636/diff/3-4/
Testing
-------
Thanks,
Jason Phelps
Re: Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
Posted by Jason Phelps via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65636/
-----------------------------------------------------------
(Updated March 29, 2019, 1 p.m.)
Review request for oozie.
Changes
-------
Rebased patch which includes the changes made in OOZIE-3370
Repository: oozie-git
Description
-------
Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java 2862d33f
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 70b9adc1
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 2bc0baa4
Diff: https://reviews.apache.org/r/65636/diff/3/
Changes: https://reviews.apache.org/r/65636/diff/2-3/
Testing
-------
Thanks,
Jason Phelps
Re: Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
Posted by Jason Phelps via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65636/
-----------------------------------------------------------
(Updated Feb. 14, 2018, 1:03 p.m.)
Review request for oozie.
Changes
-------
Had the scenerio done, but forgot the assertion test. Added it in
Repository: oozie-git
Description
-------
Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java e581e50
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java a4d3e2f
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java e71a662
Diff: https://reviews.apache.org/r/65636/diff/2/
Changes: https://reviews.apache.org/r/65636/diff/1-2/
Testing
-------
Thanks,
Jason Phelps
Re: Review Request 65636: OOZIE-3179 - Adding a configurable
config-default.xml location to a workflow
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/65636/#review197491
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java
Line 423 (original), 506-518 (patched)
<https://reviews.apache.org/r/65636/#comment277654>
I don't see the case where the same property is set in extra-config-default.xml and config-default.xml as well and config-default.xml takes precedence
- Peter Cseh
On Feb. 13, 2018, 3:07 p.m., Jason Phelps wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65636/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2018, 3:07 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Added another client property: `oozie.default.configuration.path` which allows for a comma seperated list of HDFS XML property files to be applied to the job. This is useful if multiple workflows have shared properties (i.e. database names, table names for Sqoop or Hive, spark arguments, etc). This is similar to the `oozie.libpath` property which allows user's to define common paths workflows to load jars.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java e581e50
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java a4d3e2f
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java e71a662
>
>
> Diff: https://reviews.apache.org/r/65636/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Phelps
>
>