You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/01/13 16:20:16 UTC
[GitHub] [pulsar] codelipenghui commented on a change in pull request #13745: Fix call sync method in async rest API for ``internalGetSubscriptionsForNonPartitionedTopics``
codelipenghui commented on a change in pull request #13745:
URL: https://github.com/apache/pulsar/pull/13745#discussion_r784115015
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -1137,26 +1137,27 @@ 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)
+ .thenRun(() -> validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS))
+ .thenCompose(__ -> getTopicReferenceAsync(topicName))
Review comment:
```suggestion
.thenCompose(__ -> {
validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS);
return getTopicReferenceAsync(topicName);
})
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -1137,26 +1137,27 @@ 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)
+ .thenRun(() -> validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS))
+ .thenCompose(__ -> getTopicReferenceAsync(topicName))
+ .thenAccept(topic -> {
+ final List<String> subscriptions = new ArrayList<>(topic.getSubscriptions().keys());
+ asyncResponse.resume(subscriptions);
+ }).exceptionally(ex -> {
+ Throwable cause = ex.getCause();
+ if (ex instanceof WebApplicationException) {
+ if (log.isDebugEnabled() && ((WebApplicationException) cause).getResponse().getStatus()
+ == Status.TEMPORARY_REDIRECT.getStatusCode()) {
+ log.debug("[{}] Failed to get subscriptions for non-partitioned topic {},"
+ + " redirecting to other brokers.",
+ clientAppId(), topicName, ex);
+ }
+ } else {
+ log.error("[{}] Failed to get list of subscriptions for {}", clientAppId(), topicName, ex);
+ }
+ resumeAsyncResponseExceptionally(asyncResponse, ex);
Review comment:
```suggestion
resumeAsyncResponseExceptionally(asyncResponse, cause);
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -1137,26 +1137,27 @@ 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)
+ .thenRun(() -> validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS))
+ .thenCompose(__ -> getTopicReferenceAsync(topicName))
+ .thenAccept(topic -> {
+ final List<String> subscriptions = new ArrayList<>(topic.getSubscriptions().keys());
+ asyncResponse.resume(subscriptions);
+ }).exceptionally(ex -> {
+ Throwable cause = ex.getCause();
+ if (ex instanceof WebApplicationException) {
+ if (log.isDebugEnabled() && ((WebApplicationException) cause).getResponse().getStatus()
+ == Status.TEMPORARY_REDIRECT.getStatusCode()) {
+ log.debug("[{}] Failed to get subscriptions for non-partitioned topic {},"
+ + " redirecting to other brokers.",
+ clientAppId(), topicName, ex);
Review comment:
```suggestion
log.debug("[{}] Failed to get subscriptions for non-partitioned topic {},"
+ " redirecting to other brokers.",
clientAppId(), topicName, cause);
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -1137,26 +1137,27 @@ 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)
+ .thenRun(() -> validateTopicOperation(topicName, TopicOperation.GET_SUBSCRIPTIONS))
+ .thenCompose(__ -> getTopicReferenceAsync(topicName))
+ .thenAccept(topic -> {
+ final List<String> subscriptions = new ArrayList<>(topic.getSubscriptions().keys());
+ asyncResponse.resume(subscriptions);
+ }).exceptionally(ex -> {
+ Throwable cause = ex.getCause();
+ if (ex instanceof WebApplicationException) {
+ if (log.isDebugEnabled() && ((WebApplicationException) cause).getResponse().getStatus()
+ == Status.TEMPORARY_REDIRECT.getStatusCode()) {
+ log.debug("[{}] Failed to get subscriptions for non-partitioned topic {},"
+ + " redirecting to other brokers.",
+ clientAppId(), topicName, ex);
+ }
+ } else {
+ log.error("[{}] Failed to get list of subscriptions for {}", clientAppId(), topicName, ex);
Review comment:
```suggestion
log.error("[{}] Failed to get list of subscriptions for {}", clientAppId(), topicName, cause);
```
--
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