[GitHub] [pulsar] gaoran10 commented on a change in pull request #13745: Fix call sync method in async rest API for ``internalGetSubscriptionsForNonPartitionedTopics``

[GitBox <gi...@apache.org>]

gaoran10 commented on a change in pull request #13745:
URL: https://github.com/apache/pulsar/pull/13745#discussion_r784475519



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -1137,26 +1137,29 @@ private void resumeAsyncResponse(AsyncResponse asyncResponse, Set<String> subscr
     }
 
     private void internalGetSubscriptionsForNonPartitionedTopic(AsyncResponse asyncResponse, boolean authoritative) {
-        try {
-            validateTopicOwnership(topicName, authoritative);
-            validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS);
-
-            Topic topic = getTopicReference(topicName);
-            final List<String> subscriptions = Lists.newArrayList();
-            topic.getSubscriptions().forEach((subName, sub) -> subscriptions.add(subName));
-            asyncResponse.resume(subscriptions);
-        } catch (WebApplicationException wae) {
-            if (log.isDebugEnabled()) {
-                log.debug("[{}] Failed to get subscriptions for non-partitioned topic {},"
-                                + " redirecting to other brokers.",
-                        clientAppId(), topicName, wae);
-            }
-            resumeAsyncResponseExceptionally(asyncResponse, wae);
-            return;
-        } catch (Exception e) {
-            log.error("[{}] Failed to get list of subscriptions for {}", clientAppId(), topicName, e);
-            resumeAsyncResponseExceptionally(asyncResponse, e);
-        }
+        validateTopicOwnershipAsync(topicName, authoritative)
+                .thenCompose(__ -> {
+                    validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS);
+                    return getTopicReferenceAsync(topicName);
+                })
+                .thenAccept(topic -> {
+                    final List<String> subscriptions = new ArrayList<>(topic.getSubscriptions().keys());
+                    asyncResponse.resume(subscriptions);
+                }).exceptionally(ex -> {
+                    Throwable cause = ex.getCause();
+                    if (cause instanceof WebApplicationException) {
+                        if (log.isDebugEnabled() && ((WebApplicationException) cause).getResponse().getStatus()

Review comment:
       If the status is not `Status.TEMPORARY_REDIRECT`, maybe we need to log the error, it's not normal behavior.




-- 
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-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org