[ 
https://issues.apache.org/jira/browse/DRILL-5547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16083199#comment-16083199
 ] 

ASF GitHub Bot commented on DRILL-5547:
---------------------------------------

Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/868#discussion_r126834151
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
 ---
    @@ -314,6 +370,31 @@ public void deleteAllOptions(OptionType type) {
         }
       }
     
    +  public void populateDefualtValues(Map<String, OptionValidator> 
VALIDATORS) {
    +
    +    // populate the options from the config
    +    final Map<String, OptionValidator> tmp = new HashMap<>();
    +    for (final Map.Entry<String, OptionValidator> entry : 
VALIDATORS.entrySet()) {
    +
    +      OptionValidator validator = entry.getValue();
    +      final OptionValue.Kind kind = validator.getKind();
    +      String name = entry.getValue().getOptionName();
    +      OptionValue value;
    +      String configPath = "drill.exec.options.";
    +      String configName = configPath + name;
    +      try {
    +        value = validator.loadConfigDefault(bootConfig,name,configPath);
    +        validator.setDefaultValue(value);
    +        tmp.put(name, validator);
    +      } catch (ConfigException.Missing e) {
    +        logger.error(e.getMessage(), e);
    --- End diff --
    
    Seems we'd want to throw a fatal exception here, such as 
`IllegalStateException` because this indicates, to the developers, that the 
developer forgot to externalize the config default when they created the 
validator.


> Drill config options and session options do not work as intended
> ----------------------------------------------------------------
>
>                 Key: DRILL-5547
>                 URL: https://issues.apache.org/jira/browse/DRILL-5547
>             Project: Apache Drill
>          Issue Type: Bug
>          Components:  Server
>    Affects Versions: 1.10.0
>            Reporter: Karthikeyan Manivannan
>            Assignee: Venkata Jyothsna Donapati
>             Fix For: Future
>
>
> In Drill, session options should take precedence over config options. But 
> several of these session options are assigned hard-coded default values when 
> the option validators are initialized. Because of this config options will 
> never be read and honored even if the user did not specify the session 
> option. 
> ClassCompilerSelector.JAVA_COMPILER_VALIDATOR uses CompilerPolicy.DEFAULT as 
> the default value. This default value gets into the session options map via 
> the initialization of validators in SystemOptionManager. 
> Now any piece of code that tries to check if a session option is set will 
> never see a null, so it will always use that value and never try to look into 
> the config options. For example, in the following piece of code from 
> ClassCompilerSelector (), the policy will never be read from the config file.
> policy = CompilerPolicy.valueOf((value != null) ? 
> value.string_val.toUpperCase() : 
> config.getString(JAVA_COMPILER_CONFIG).toUpperCase());



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to