You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Abhishek Bafna <ba...@gmail.com> on 2016/07/23 04:58:58 UTC
Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/
-----------------------------------------------------------
Review request for oozie.
Bugs: OOZIE-2583
https://issues.apache.org/jira/browse/OOZIE-2583
Repository: oozie-git
Description
-------
OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
Diffs
-----
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
Diff: https://reviews.apache.org/r/50372/diff/
Testing
-------
Unit test.
Thanks,
Abhishek Bafna
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Abhishek Bafna <ba...@gmail.com>.
> On July 26, 2016, 9 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java, line 169
> > <https://reviews.apache.org/r/50372/diff/1/?file=1451549#file1451549line169>
> >
> > Would it makes sense to log when this (rare) situation happens? It could be useful for later analysis.
Added an INFO level log, every time config-default is resolved from workflow conf. As these cases are not so common, so should not be noise.
> On July 26, 2016, 9 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java, line 170
> > <https://reviews.apache.org/r/50372/diff/1/?file=1451549#file1451549line170>
> >
> > Can you please extract entry.getKey() to a local String variable (e.g. defaultConfKey)?
Done.
> On July 26, 2016, 9 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java, line 163
> > <https://reviews.apache.org/r/50372/diff/1/?file=1451549#file1451549line163>
> >
> > Can you extract this part into a new method , something like private Configuration resolveVariablesFromConfigDefault(Configuration defaultConf)?
> >
> > Then you could just
> > defaultConf = resolveVariablesFromConfigDefault(defaultConf);
Done.
- Abhishek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/#review143499
-----------------------------------------------------------
On July 23, 2016, 4:58 a.m., Abhishek Bafna wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50372/
> -----------------------------------------------------------
>
> (Updated July 23, 2016, 4:58 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2583
> https://issues.apache.org/jira/browse/OOZIE-2583
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
>
> Diff: https://reviews.apache.org/r/50372/diff/
>
>
> Testing
> -------
>
> Unit test.
>
>
> Thanks,
>
> Abhishek Bafna
>
>
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/#review143499
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java (line 163)
<https://reviews.apache.org/r/50372/#comment209312>
Can you extract this part into a new method , something like private Configuration resolveVariablesFromConfigDefault(Configuration defaultConf)?
Then you could just
defaultConf = resolveVariablesFromConfigDefault(defaultConf);
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java (line 169)
<https://reviews.apache.org/r/50372/#comment209311>
Would it makes sense to log when this (rare) situation happens? It could be useful for later analysis.
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java (line 170)
<https://reviews.apache.org/r/50372/#comment209310>
Can you please extract entry.getKey() to a local String variable (e.g. defaultConfKey)?
- Attila Sasvari
On July 23, 2016, 4:58 a.m., Abhishek Bafna wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50372/
> -----------------------------------------------------------
>
> (Updated July 23, 2016, 4:58 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2583
> https://issues.apache.org/jira/browse/OOZIE-2583
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
>
> Diff: https://reviews.apache.org/r/50372/diff/
>
>
> Testing
> -------
>
> Unit test.
>
>
> Thanks,
>
> Abhishek Bafna
>
>
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Abhishek Bafna <ba...@gmail.com>.
> On July 27, 2016, 7:58 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java, line 299
> > <https://reviews.apache.org/r/50372/diff/2/?file=1452620#file1452620line299>
> >
> > It is good that we now log the conf key, but I am wondering if the associated value contains sensitive information (such as some passwords). If so, we should mask it when logging or it might be better to not log the actual value at all.
I totally missed this statement: XConfiguration.injectDefaults(defaultConf, conf); Line: 157
Which means that all the config-default.xml are in the workflow conf.
Now It can resolve all the properties. Something which can be resolved within the config-default.xml, will be done else look into the workflow conf.
Added an extra assert statement for the same in testcase.
Removed the logging.
Let me know your further thoughts on this. :)
Thanks.
- Abhishek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/#review143696
-----------------------------------------------------------
On July 26, 2016, 11:02 a.m., Abhishek Bafna wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50372/
> -----------------------------------------------------------
>
> (Updated July 26, 2016, 11:02 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2583
> https://issues.apache.org/jira/browse/OOZIE-2583
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
>
> Diff: https://reviews.apache.org/r/50372/diff/
>
>
> Testing
> -------
>
> Unit test.
>
>
> Thanks,
>
> Abhishek Bafna
>
>
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/#review143696
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java (line 299)
<https://reviews.apache.org/r/50372/#comment209602>
It is good that we now log the conf key, but I am wondering if the associated value contains sensitive information (such as some passwords). If so, we should mask it when logging or it might be better to not log the actual value at all.
- Attila Sasvari
On July 26, 2016, 11:02 a.m., Abhishek Bafna wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50372/
> -----------------------------------------------------------
>
> (Updated July 26, 2016, 11:02 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2583
> https://issues.apache.org/jira/browse/OOZIE-2583
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
>
> Diff: https://reviews.apache.org/r/50372/diff/
>
>
> Testing
> -------
>
> Unit test.
>
>
> Thanks,
>
> Abhishek Bafna
>
>
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/#review144078
-----------------------------------------------------------
Ship it!
Ship It!
- Attila Sasvari
On July 27, 2016, 9:38 a.m., Abhishek Bafna wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50372/
> -----------------------------------------------------------
>
> (Updated July 27, 2016, 9:38 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2583
> https://issues.apache.org/jira/browse/OOZIE-2583
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
> core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
>
> Diff: https://reviews.apache.org/r/50372/diff/
>
>
> Testing
> -------
>
> Unit test.
>
>
> Thanks,
>
> Abhishek Bafna
>
>
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Abhishek Bafna <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/
-----------------------------------------------------------
(Updated July 27, 2016, 9:38 a.m.)
Review request for oozie.
Bugs: OOZIE-2583
https://issues.apache.org/jira/browse/OOZIE-2583
Repository: oozie-git
Description
-------
OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
Diff: https://reviews.apache.org/r/50372/diff/
Testing
-------
Unit test.
Thanks,
Abhishek Bafna
Re: Review Request 50372: OOZIE-2583: oozie throws EL Exception when
reference variable name containing dot
Posted by Abhishek Bafna <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50372/
-----------------------------------------------------------
(Updated July 26, 2016, 11:02 a.m.)
Review request for oozie.
Bugs: OOZIE-2583
https://issues.apache.org/jira/browse/OOZIE-2583
Repository: oozie-git
Description
-------
OOZIE-2583: oozie throws EL Exception when reference variable name containing dot
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 27274b9
core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 73464c8
Diff: https://reviews.apache.org/r/50372/diff/
Testing
-------
Unit test.
Thanks,
Abhishek Bafna