Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/868#discussion_r126833530
--- 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) {
--- End diff --
Nit: the `VALIDATORS` variable is not a constant here, so we should call it
something like `validators`.
Or, since `VALIDATORS` is a static member, we can access it directly; no
need to pass it into this method.
Moreover, since `VALIDATORS` is static, this whole method can be static. It
should be called (once) from somewhere in, say, the Drillbit startup logic.
But, since we can have multiple drillbits in the same process (when testing),
we should perhaps have a flag so that initialization is done only on the first
call, and synchronized so that two Drillbits don't try to do the same init at
the same time.
Or, we might want to have a separate table per Drillbit. Maybe the items
should become a non-static member of the SessionOptions class, if we have one
SessionOptions per Drillbit. That way we can test strange scenarios, such as
two Drillbits having different defaults (which is a bug, but we might want to
test if we can catch that improper setup...)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
