----------------------------------------------------------- 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 > >