tpalfy commented on a change in pull request #5319:
URL: https://github.com/apache/nifi/pull/5319#discussion_r706209731
##########
File path:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -702,6 +714,19 @@ public void onPropertyModified(final PropertyDescriptor
descriptor, final String
.build());
}
+ boolean usingUserNamePasswordAuthorization =
validationContext.getProperty(PROP_BASIC_AUTH_USERNAME).isSet()
+ || validationContext.getProperty(PROP_BASIC_AUTH_USERNAME).isSet();
+
+ boolean usingOAuth2Authorization =
validationContext.getProperty(OAUTH2_ACCESS_TOKEN_PROVIDER).isSet();
+
+ if (usingUserNamePasswordAuthorization && usingOAuth2Authorization) {
+ results.add(new ValidationResult.Builder()
+ .subject("Authorization properties")
+ .valid(false)
+ .explanation("Can't use username+password and OAuth2
authorization at the same time")
Review comment:
I disagree. There are 3 fairly long property names that could be
referenced here ('Basic Authentication Username', 'Basic Authentication
Password' and 'OAuth2 Access Token provider') and putting them all in a fairly
convoluted manner in the explanation would make it too bloated and harder to
actually interpret the issue.
On the other hand having the explanation message be something like "Can't
use Basic- and OAuth2 authorization at the same time" would be a bit too
lacking and would require just a bit too much extra effort to figure out what
exactly the issue is.
I think the current message gives just the right amount of detail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
