drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ilooner <...@git.apache.org>
Subject [GitHub] drill issue #923: DRILL-5723: Added System Internal Options That can be Modi...
Date Thu, 07 Sep 2017 23:55:29 GMT
Github user ilooner commented on the issue:

    https://github.com/apache/drill/pull/923
  
    So it looks like Drill's treatment of OptionType and OptionScope is inconsistent at present.
To correct this I am doing the following:
    
    - OptionType seems to be I'll defined. I think the intention is that it defines the levels
in which an option can be configured. This should be a static property tied to an option (much
like a validator). In reality it looks like the OptionType of an option can be changed depending
on where it is set, which is incorrect behavior. I think this came about because it is the
user's responsibility to define the OptionType for an option value which is in correct.
    - OptionScope will define the level at which the option was set
    
    To solidify the semantics I am doing the following:
    
    - OptionManagers no longer allow an option value to be set. Only the following methods
are exposed: ```
    void setLocalOption(String name, boolean value);
    void setLocalOption(String name, long value);
    void setLocalOption(String name, double value);
    void setLocalOption(String name, String value);
    void setLocalOption(OptionValue.Kind kind, String name, String value);
    ``` Doing this prevents the possibility of defining an incorrect scope or type. The option
type is now determined by the static option definition. The option scope is determined by
the option manager on which the option was set.
    
    - I have also cleaned up the OptionManager interface so that it does not require the option
type to be passed to the delete methods. This helps clean up the semantics further because
OptionType was being overloaded to define both where an option can be set and which option
manager can set it. This is unclear because some options can be set exclusively at one level
(e.g. only the session level) whereas other options can be set at multiple levels (e.g. system
and session levels). It also introduces the possibility for error by allowing the user to
provide the wrong type to the wrong option manager.


---

Mime
View raw message