[
https://issues.apache.org/jira/browse/DRILL-5723?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16160142#comment-16160142
]
ASF GitHub Bot commented on DRILL-5723:
---------------------------------------
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/923#discussion_r137937125
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java
---
@@ -17,44 +17,84 @@
*/
package org.apache.drill.exec.server.options;
-import
org.apache.drill.exec.server.options.TypeValidators.BooleanValidator;
-import org.apache.drill.exec.server.options.TypeValidators.DoubleValidator;
-import org.apache.drill.exec.server.options.TypeValidators.LongValidator;
-import org.apache.drill.exec.server.options.TypeValidators.StringValidator;
-
-public abstract class BaseOptionManager implements OptionSet {
-// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(BaseOptionManager.class);
-
- /**
- * Gets the current option value given a validator.
- *
- * @param validator the validator
- * @return option value
- * @throws IllegalArgumentException - if the validator is not found
- */
- private OptionValue getOptionSafe(OptionValidator validator) {
- OptionValue value = getOption(validator.getOptionName());
- return value == null ? validator.getDefault() : value;
+import org.apache.drill.common.exceptions.UserException;
+
+import java.util.Iterator;
+
+/**
+ * This {@link OptionManager} implements some the basic methods and should
be extended by concrete implementations.
+ */
+public abstract class BaseOptionManager extends BaseOptionSet implements
OptionManager {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(BaseOptionManager.class);
+
+ @Override
+ public OptionList getInternalOptionList() {
+ return getAllOptionList(true);
}
@Override
- public boolean getOption(BooleanValidator validator) {
- return getOptionSafe(validator).bool_val;
+ public OptionList getPublicOptionList() {
+ return getAllOptionList(false);
}
@Override
- public double getOption(DoubleValidator validator) {
- return getOptionSafe(validator).float_val;
+ public void setLocalOption(String name, boolean value) {
+ setLocalOption(OptionValue.Kind.BOOLEAN, name,
Boolean.toString(value));
}
@Override
- public long getOption(LongValidator validator) {
- return getOptionSafe(validator).num_val;
+ public void setLocalOption(String name, long value) {
+ setLocalOption(OptionValue.Kind.LONG, name, Long.toString(value));
}
@Override
- public String getOption(StringValidator validator) {
- return getOptionSafe(validator).string_val;
+ public void setLocalOption(String name, double value) {
+ setLocalOption(OptionValue.Kind.DOUBLE, name, Double.toString(value));
}
+ @Override
+ public void setLocalOption(String name, String value) {
+ setLocalOption(OptionValue.Kind.STRING, name, value);
+ }
+
+ @Override
+ public void setLocalOption(OptionValue.Kind kind, String name, String
value) {
+ final OptionDefinition definition = getOptionDefinition(name);
+ final OptionValidator validator = definition.getValidator();
+ final OptionMetaData metaData = definition.getMetaData();
+ final OptionValue.OptionType type = definition.getMetaData().getType();
+ final OptionValue.OptionScope scope = getScope();
+ checkOptionPermissions(name, type, scope);
+ final OptionValue optionValue = OptionValue.create(kind, type, name,
value, scope);
+ validator.validate(optionValue, metaData, this);
+ setLocalOptionHelper(optionValue);
--- End diff --
Very nice and clean, does all the necessary steps in one place. Great
improvement!
> 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)
