brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #790: Adds HttpFeed.preemptiveBasicAuth
Date Fri, 04 Aug 2017 09:39:12 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/790#discussion_r131351303
  
    --- Diff: core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java ---
    @@ -228,6 +235,25 @@ public Builder httpExecutor(HttpExecutor val) {
                 this.httpExecutor = val;
                 return this;
             }
    +        public Map<String, String> buildBaseHeaders() {
    +            if (Boolean.TRUE.equals(preemptiveBasicAuth)) {
    +                Credentials creds = credentials;
    +                if (creds == null) {
    +                    throw new IllegalArgumentException("Must not enable preemptiveBasicAuth
when there are no credentials, in feed for "+baseUri);
    --- End diff --
    
    My personal preference is to create new exception types when it is useful to the user.
But saying that, if it extends `IllegalArgumentException` then there's not much difference
from their perspective.
    
    If we created things as specific as `MisconfiguredPreemptiveAuthException`, I worry that
this leads towards the extreme of an exception per config key!
    
    Maybe a middle ground is to have an `IllegalConfigurationException` or some such?
    
    I'm personally ok with us asserting exception message snippets in unit tests (they change
rarely, and it confirms the message does appear at the top level rather than only being buried
far down in the "caused by").
    
    However, the text in this particular example is far too long!
    
    Perhaps we should discuss this separately, and figure out our approach to such exception
across-the-board.


---
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