drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jacques-n <...@git.apache.org>
Subject [GitHub] drill pull request: DRILL-1065: Support for ALTER ... RESET statem...
Date Thu, 01 Oct 2015 05:48:24 GMT
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/159#discussion_r40880950
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
---
    @@ -102,6 +125,29 @@ public void setOption(OptionValue value) {
       }
     
       @Override
    +  public void deleteOption(final String name, final OptionType type) {
    +    try {
    +      SystemOptionManager.getValidator(name); // ensure the option exists
    +    } catch (final IllegalArgumentException e) {
    +      throw UserException.validationError(e)
    +        .build(logger);
    --- End diff --
    
    My main issue here is that we have an unfriendly message. One of the things that we should
be doing is creating a user friendly message at the point of user exception creation. If you
think it was too deep one level deeper, then you should make a friendly user message at this
level. Trusting that the system level exception message is meaningful to the user should be
avoided.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message