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 [email protected] or file a JIRA ticket
with INFRA.
---
