eolivelli commented on a change in pull request #9308:
URL: https://github.com/apache/pulsar/pull/9308#discussion_r598651697
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/MessagingServiceShutdownHook.java
##########
@@ -59,9 +59,14 @@ public void run() {
executor.execute(() -> {
try {
- service.close();
- future.complete(null);
- } catch (PulsarServerException e) {
+ service.closeAsync().whenComplete((result, throwable) -> {
+ if (throwable != null) {
+ future.completeExceptionally(throwable);
+ } else {
+ future.complete(result);
+ }
+ });
+ } catch (Exception e) {
Review comment:
Sorry, I don't want to be so picky, but "catch (Exception e)" is really
something we do not want to have if not strictly necessary..
It totally fine to me to handle the result with `whenComplete`, this is
expected when you deal with CompletableFuture.
My problem is "catch Exception" around a method call to something that
returns a CompletableFuture.
the only way to get rid of the "catch Exception" is to remove the "throws
PulsarServerException" clause from closeAsync.
`closeAsync` is a new method, so we can define the signature without the
"throws" clause
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
