mattisonchao commented on code in PR #15760:
URL: https://github.com/apache/pulsar/pull/15760#discussion_r884358141


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -379,10 +388,20 @@ public Set<String> 
getNamespaceReplicationClusters(@PathParam("property") String
             @ApiResponse(code = 404, message = "Property or cluster or 
namespace doesn't exist"),
             @ApiResponse(code = 409, message = "Peer-cluster can't be part of 
replication-cluster"),
             @ApiResponse(code = 412, message = "Namespace is not global or 
invalid cluster ids") })
-    public void setNamespaceReplicationClusters(@PathParam("property") String 
property,
-            @PathParam("cluster") String cluster, @PathParam("namespace") 
String namespace, List<String> clusterIds) {
+    public void setNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("property") String 
property,
+                                                @PathParam("cluster") String 
cluster,
+                                                @PathParam("namespace") String 
namespace, List<String> clusterIds) {
         validateNamespaceName(property, cluster, namespace);
-        internalSetNamespaceReplicationClusters(clusterIds);
+        internalSetNamespaceReplicationClusters(clusterIds)
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);
+                    log.error("[{}] Field to set namespace replication 
clusters on namespace {}", clientAppId(),

Review Comment:
   typo



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -308,10 +308,19 @@ public void 
revokePermissionOnSubscription(@PathParam("property") String propert
     @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have 
admin permission"),
             @ApiResponse(code = 404, message = "Tenant or cluster or namespace 
doesn't exist"),
             @ApiResponse(code = 412, message = "Namespace is not global")})
-    public Set<String> getNamespaceReplicationClusters(@PathParam("tenant") 
String tenant,
-            @PathParam("namespace") String namespace) {
+    public void getNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("tenant") String 
tenant,
+                                                @PathParam("namespace") String 
namespace) {
         validateNamespaceName(tenant, namespace);
-        return internalGetNamespaceReplicationClusters();
+        internalGetNamespaceReplicationClustersAsync()
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);
+                    log.error("[{}] Field to get namespace replication 
clusters on namespace {}", clientAppId(),

Review Comment:
   typo



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -379,10 +388,20 @@ public Set<String> 
getNamespaceReplicationClusters(@PathParam("property") String
             @ApiResponse(code = 404, message = "Property or cluster or 
namespace doesn't exist"),
             @ApiResponse(code = 409, message = "Peer-cluster can't be part of 
replication-cluster"),
             @ApiResponse(code = 412, message = "Namespace is not global or 
invalid cluster ids") })
-    public void setNamespaceReplicationClusters(@PathParam("property") String 
property,
-            @PathParam("cluster") String cluster, @PathParam("namespace") 
String namespace, List<String> clusterIds) {
+    public void setNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("property") String 
property,
+                                                @PathParam("cluster") String 
cluster,
+                                                @PathParam("namespace") String 
namespace, List<String> clusterIds) {
         validateNamespaceName(property, cluster, namespace);
-        internalSetNamespaceReplicationClusters(clusterIds);
+        internalSetNamespaceReplicationClusters(clusterIds)
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);

Review Comment:
   looks like we don't need this line.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -363,13 +363,22 @@ public void 
revokePermissionOnSubscription(@PathParam("property") String propert
     @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have 
admin permission"),
             @ApiResponse(code = 404, message = "Property or cluster or 
namespace doesn't exist"),
             @ApiResponse(code = 412, message = "Namespace is not global")})
-    public Set<String> getNamespaceReplicationClusters(@PathParam("property") 
String property,
-            @PathParam("cluster") String cluster, @PathParam("namespace") 
String namespace) {
+    public void getNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("property") String 
property,
+                                                @PathParam("cluster") String 
cluster,
+                                                @PathParam("namespace") String 
namespace) {
         validateNamespaceName(property, cluster, namespace);
-        validateNamespacePolicyOperation(NamespaceName.get(property, 
namespace),
-                PolicyName.REPLICATION, PolicyOperation.READ);
-
-        return internalGetNamespaceReplicationClusters();
+        validateNamespacePolicyOperationAsync(NamespaceName.get(property, 
namespace),
+                PolicyName.REPLICATION, PolicyOperation.READ)
+                .thenCompose(__ -> 
internalGetNamespaceReplicationClustersAsync())
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);

Review Comment:
   looks like we don't need this line.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -363,13 +363,22 @@ public void 
revokePermissionOnSubscription(@PathParam("property") String propert
     @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have 
admin permission"),
             @ApiResponse(code = 404, message = "Property or cluster or 
namespace doesn't exist"),
             @ApiResponse(code = 412, message = "Namespace is not global")})
-    public Set<String> getNamespaceReplicationClusters(@PathParam("property") 
String property,
-            @PathParam("cluster") String cluster, @PathParam("namespace") 
String namespace) {
+    public void getNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("property") String 
property,
+                                                @PathParam("cluster") String 
cluster,
+                                                @PathParam("namespace") String 
namespace) {
         validateNamespaceName(property, cluster, namespace);
-        validateNamespacePolicyOperation(NamespaceName.get(property, 
namespace),
-                PolicyName.REPLICATION, PolicyOperation.READ);
-
-        return internalGetNamespaceReplicationClusters();
+        validateNamespacePolicyOperationAsync(NamespaceName.get(property, 
namespace),
+                PolicyName.REPLICATION, PolicyOperation.READ)
+                .thenCompose(__ -> 
internalGetNamespaceReplicationClustersAsync())
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);
+                    log.error("[{}] Field to get namespace replication 
clusters on namespace {}", clientAppId(),

Review Comment:
   typo



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -308,10 +308,19 @@ public void 
revokePermissionOnSubscription(@PathParam("property") String propert
     @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have 
admin permission"),
             @ApiResponse(code = 404, message = "Tenant or cluster or namespace 
doesn't exist"),
             @ApiResponse(code = 412, message = "Namespace is not global")})
-    public Set<String> getNamespaceReplicationClusters(@PathParam("tenant") 
String tenant,
-            @PathParam("namespace") String namespace) {
+    public void getNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("tenant") String 
tenant,
+                                                @PathParam("namespace") String 
namespace) {
         validateNamespaceName(tenant, namespace);
-        return internalGetNamespaceReplicationClusters();
+        internalGetNamespaceReplicationClustersAsync()
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);

Review Comment:
   looks like we don't need this line.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -321,11 +330,20 @@ public Set<String> 
getNamespaceReplicationClusters(@PathParam("tenant") String t
             @ApiResponse(code = 404, message = "Tenant or cluster or namespace 
doesn't exist"),
             @ApiResponse(code = 409, message = "Peer-cluster can't be part of 
replication-cluster"),
             @ApiResponse(code = 412, message = "Namespace is not global or 
invalid cluster ids") })
-    public void setNamespaceReplicationClusters(@PathParam("tenant") String 
tenant,
-            @PathParam("namespace") String namespace,
+    public void setNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("tenant") String 
tenant,
+                                                @PathParam("namespace") String 
namespace,
             @ApiParam(value = "List of replication clusters", required = true) 
List<String> clusterIds) {
         validateNamespaceName(tenant, namespace);
-        internalSetNamespaceReplicationClusters(clusterIds);
+        internalSetNamespaceReplicationClusters(clusterIds)
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);

Review Comment:
   looks like we don't need this line. because 
`resumeAsyncResponseExceptionally` already does that.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -321,11 +330,20 @@ public Set<String> 
getNamespaceReplicationClusters(@PathParam("tenant") String t
             @ApiResponse(code = 404, message = "Tenant or cluster or namespace 
doesn't exist"),
             @ApiResponse(code = 409, message = "Peer-cluster can't be part of 
replication-cluster"),
             @ApiResponse(code = 412, message = "Namespace is not global or 
invalid cluster ids") })
-    public void setNamespaceReplicationClusters(@PathParam("tenant") String 
tenant,
-            @PathParam("namespace") String namespace,
+    public void setNamespaceReplicationClusters(@Suspended AsyncResponse 
asyncResponse,
+                                                @PathParam("tenant") String 
tenant,
+                                                @PathParam("namespace") String 
namespace,
             @ApiParam(value = "List of replication clusters", required = true) 
List<String> clusterIds) {
         validateNamespaceName(tenant, namespace);
-        internalSetNamespaceReplicationClusters(clusterIds);
+        internalSetNamespaceReplicationClusters(clusterIds)
+                .thenAccept(asyncResponse::resume)
+                .exceptionally(e -> {
+                    Throwable throwable = 
FutureUtil.unwrapCompletionException(e);
+                    log.error("[{}] Field to set namespace replication 
clusters on namespace {}",

Review Comment:
   typo



-- 
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]

Reply via email to