oneby-wang commented on code in PR #24714:
URL: https://github.com/apache/pulsar/pull/24714#discussion_r2331753787
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java:
##########
@@ -219,7 +219,7 @@ public CompletableFuture<Boolean> canProduceAsync(TopicName
topicName, String ro
if (!this.conf.isAuthorizationEnabled()) {
return CompletableFuture.completedFuture(true);
}
- return provider.isSuperUser(role, authenticationData,
conf).thenComposeAsync(isSuperUser -> {
+ return provider.isSuperUser(role, authenticationData,
conf).thenCompose(isSuperUser -> {
Review Comment:
I would assume that the implementer of the interface should implement the
async semantics, rather than using the thenComposeAsync method to ensure that
custom methods without async implementation can run asynchronously.
https://github.com/apache/pulsar/blob/c7dff63e8d38bfdc7ac881b5fec8c1b486346e6e/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java#L112-L113
My opinion: it is the duty of implementer implement the async semantics to
make caller simpler, rather than leaving it to the caller to handle. At least
currently, both implementations in Pulsar are non-blocking, the async operation
is called by MetaCache executor.
https://github.com/apache/pulsar/blob/c7dff63e8d38bfdc7ac881b5fec8c1b486346e6e/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/NamespaceResources.java#L136-L138
If we need to consider such an assumption, then some places in the project
may need to be replaced with thenComposeAsync method.
--
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: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
