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

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

Github user ilooner commented on a diff in the pull request:

    https://github.com/apache/drill/pull/923#discussion_r138168825
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
 ---
    @@ -17,49 +17,97 @@
      */
     package org.apache.drill.exec.server.options;
     
    -import org.apache.drill.exec.server.options.OptionValue.OptionType;
    +import javax.validation.constraints.NotNull;
     
     /**
      * Manager for Drill {@link OptionValue options}. Implementations must be 
case-insensitive to the name of an option.
      */
     public interface OptionManager extends OptionSet, Iterable<OptionValue> {
     
       /**
    -   * Sets an option value.
    -   *
    -   * @param value option value
    -   * @throws org.apache.drill.common.exceptions.UserException message to 
describe error with value
    +   * Sets a boolean option on the {@link OptionManager}.
    +   * @param name The name of the option.
    +   * @param value The value of the option.
        */
    -  void setOption(OptionValue value);
    +  void setLocalOption(String name, boolean value);
     
       /**
    -   * Deletes the option. Unfortunately, the type is required given the 
fallback structure of option managers.
    -   * See {@link FallbackOptionManager}.
    +   * Sets a long option on the {@link OptionManager}.
    +   * @param name The name of the option.
    +   * @param value The value of the option.
    +   */
    +  void setLocalOption(String name, long value);
    +
    +  /**
    +   * Sets a double option on the {@link OptionManager}.
    +   * @param name The name of the option.
    +   * @param value The value of the option.
    +   */
    +  void setLocalOption(String name, double value);
    +
    +  /**
    +   * Sets a String option on the {@link OptionManager}.
    +   * @param name The name of the option.
    +   * @param value The value of the option.
    +   */
    +  void setLocalOption(String name, String value);
    +
    +  /**
    +   * Sets an option of the specified {@link OptionValue.Kind} on the 
{@link OptionManager}.
    +   * @param kind The kind of the option.
    +   * @param name The name of the option.
    +   * @param value The value of the option.
    +   */
    +  void setLocalOption(OptionValue.Kind kind, String name, String value);
    +
    +  /**
    +   * Deletes the option.
        *
    -   * If the option name is valid (exists in {@link 
SystemOptionManager#VALIDATORS}),
    +   * If the option name is valid (exists in the set of validators produced 
by {@link SystemOptionManager#createDefaultOptionDefinitions()}),
        * but the option was not set within this manager, calling this method 
should be a no-op.
        *
        * @param name option name
    -   * @param type option type
        * @throws org.apache.drill.common.exceptions.UserException message to 
describe error with value
        */
    -  void deleteOption(String name, OptionType type);
    +  void deleteLocalOption(String name);
     
       /**
    -   * Deletes all options. Unfortunately, the type is required given the 
fallback structure of option managers.
    -   * See {@link FallbackOptionManager}.
    +   * Deletes all options.
        *
        * If no options are set, calling this method should be no-op.
        *
    -   * @param type option type
        * @throws org.apache.drill.common.exceptions.UserException message to 
describe error with value
        */
    -  void deleteAllOptions(OptionType type);
    +  void deleteAllLocalOptions();
    +
    +  /**
    +   * Get the option definition corresponding to the given option name.
    +   * @param name The name of the option to retrieve a validator for.
    +   * @return The option validator corresponding to the given option name.
    +   */
    +  @NotNull
    +  OptionDefinition getOptionDefinition(String name);
     
       /**
        * Gets the list of options managed this manager.
        *
        * @return the list of options
        */
       OptionList getOptionList();
    +
    +  /**
    +   * Returns all the internal options contained in this option manager.
    +   *
    +   * @return All the internal options contained in this option manager.
    +   */
    +  @NotNull
    +  OptionList getInternalOptionList();
    --- End diff --
    
    Internal and Local are not the same thing. Local means that the value for 
the option is stored in this option manager. Internal means that it is not 
visible with the rest of the options unless you look in the special 
sys.internal_options table. I will add this distinction to the javadoc.


> Support System/Session Internal Options And Additional Option System Fixes
> --------------------------------------------------------------------------
>
>                 Key: DRILL-5723
>                 URL: https://issues.apache.org/jira/browse/DRILL-5723
>             Project: Apache Drill
>          Issue Type: New Feature
>            Reporter: Timothy Farkas
>            Assignee: Timothy Farkas
>
> This is a feature proposed by [~ben-zvi].
> Currently all the options are accessible by the user in sys.options. We would 
> like to add internal options which can be altered, but are not visible in the 
> sys.options table. These internal options could be seen by another alias 
> select * from internal.options. The intention would be to put new options we 
> weren't comfortable with exposing to the end user in this table.
> After the options and their corresponding features are considered stable they 
> could be changed to appear in the sys.option table.
> A bunch of other fixes to the Option system have been clubbed into this:
> * OptionValidators no longer hold default values. Default values are 
> contained in the SystemOptionManager
> * Options have an OptionDefinition. The option definition includes:
>   * A validator
>   * Metadata about the options visibility, required permissions, and the 
> scope in which it can be set.
> * The Option Manager interface has been cleaned up so that a Type is not 
> required to be passed in in order to set and delete options



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

Reply via email to