michaeljmarshall commented on code in PR #19412:
URL: https://github.com/apache/pulsar/pull/19412#discussion_r1096264940
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java:
##########
@@ -140,7 +140,7 @@ public CompletableFuture<Boolean> canConsumeAsync(TopicName
topicName, String ro
}).exceptionally(ex -> {
log.warn("Client with Role - {} failed to get permissions
for topic - {}. {}", role, topicName,
ex.getMessage());
- return null;
+ throw FutureUtil.wrapToCompletionException(ex);
Review Comment:
We already log the "error" multiple times. We log it in this class as a
warning. We also have these logs for handling the result of this method:
https://github.com/apache/pulsar/blob/952187279227fbf46af0f68d31ede1f5567e8075/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java#L888-L902
After this change, we could have 3 lines and a stack trace. I think that is
too many. I would prefer to simplify the code path and only log a single
failure/error/rejection once.
--
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]
