-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66305/#review200046
-----------------------------------------------------------



Interesting idea!

However, in the existing approach within `ConfigurationService#loadConf()` 
following handling of system properties is implemented:

* `ConfigurationService#IGNORE_SYS_PROPS` defines the system properties that 
should never be overwritten, neither from config file nor from `-D` option
* `ConfigurationService#IGNORE_SYS_PROPS` contains both hard coded and 
additionally configured entries
* `ConfigurationService#CONF_SYS_PROPS` defines a fixed set of system 
properties. When the actual system property is present, and isn't contained in 
`ConfigurationService#IGNORE_SYS_PROPS`, a `Configuration` entry is inserted
* in addition to `ConfigurationService#CONF_SYS_PROPS`, all the already 
existing `Configuration` entries are checked for existence as a system 
property. If present and not contained in 
`ConfigurationService#IGNORE_SYS_PROPS`, the appropriare `Configuration` entry 
is updated

I'd suggest that the handling of the extra, admin-allowed system properties 
happens as the last step, and an extra filtering should also be taken for 
absence inside `ConfigurationService#IGNORE_SYS_PROPS`. The prefix should also 
be `oozie.`

- András Piros


On March 27, 2018, 11:59 a.m., Denes Bodo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66305/
> -----------------------------------------------------------
> 
> (Updated March 27, 2018, 11:59 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Bugs: OOZIE-3199
>     https://issues.apache.org/jira/browse/OOZIE-3199
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> Let system property restriction configurable
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/service/ConfigurationService.java 
> 618d5e6 
>   core/src/main/resources/oozie-default.xml 6f89258 
> 
> 
> Diff: https://reviews.apache.org/r/66305/diff/1/
> 
> 
> Testing
> -------
> 
> Tested on private Hadoop cluster.
> 
> 
> Thanks,
> 
> Denes Bodo
> 
>

Reply via email to