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 to implement the async semantics
to make caller simpler, rather than leaving it to the caller to handle. At
least currently, both implementations(PulsarAuthorizationProvider and
MultiRolesTokenAuthorizationProvider) in Pulsar are non-blocking, the async
operation is hold 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
