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 2020/11/14 00:16:36 UTC

[GitHub] [pulsar] rdhabalia opened a new pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

rdhabalia opened a new pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565


   ### Motivation
   Right now, Topic level dedup policy configuration doesn't support V1 topic with cluster name in it.
   
   **NOTE:** This is PR is on top of: #8555


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on a change in pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#discussion_r524873416



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
##########
@@ -347,6 +348,67 @@ public void getPartitionedStats(@Suspended final AsyncResponse asyncResponse,
         }
     }
 
+    @GET
+    @Path("/{tenant}/{cluster}/{namespace}/{topic}/deduplicationEnabled")
+    @ApiOperation(value = "Get deduplication configuration of a topic.")
+    @ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have admin permission"),
+            @ApiResponse(code = 404, message = "Tenant or cluster or namespace or topic doesn't exist"),
+            @ApiResponse(code = 405, message = "Topic level policy is disabled, to enable the topic level policy and retry")})
+    public void getDeduplicationEnabled(@Suspended final AsyncResponse asyncResponse,
+                             @PathParam("tenant") String tenant,
+                             @PathParam("cluster") String cluster,
+                             @PathParam("namespace") String namespace,
+                             @PathParam("topic") @Encoded String encodedTopic) {
+        validateTopicName(tenant, cluster, namespace, encodedTopic);
+        TopicPolicies topicPolicies = getTopicPolicies(topicName).orElse(new TopicPolicies());

Review comment:
       Should we check the topic ownership check?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] rdhabalia commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-727289729


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] rdhabalia commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-727143221


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] zymap commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
zymap commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-785514046


   Because this PR is opened for a long time. I will close this PR. Feel free to reopen it if you needed.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] rdhabalia commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-728693191






----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-731489169


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] sijie commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-730525972


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] zymap closed pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
zymap closed pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] zymap commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
zymap commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-727715825






----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] rdhabalia commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-727120463


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on a change in pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#discussion_r524873416



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
##########
@@ -347,6 +348,67 @@ public void getPartitionedStats(@Suspended final AsyncResponse asyncResponse,
         }
     }
 
+    @GET
+    @Path("/{tenant}/{cluster}/{namespace}/{topic}/deduplicationEnabled")
+    @ApiOperation(value = "Get deduplication configuration of a topic.")
+    @ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have admin permission"),
+            @ApiResponse(code = 404, message = "Tenant or cluster or namespace or topic doesn't exist"),
+            @ApiResponse(code = 405, message = "Topic level policy is disabled, to enable the topic level policy and retry")})
+    public void getDeduplicationEnabled(@Suspended final AsyncResponse asyncResponse,
+                             @PathParam("tenant") String tenant,
+                             @PathParam("cluster") String cluster,
+                             @PathParam("namespace") String namespace,
+                             @PathParam("topic") @Encoded String encodedTopic) {
+        validateTopicName(tenant, cluster, namespace, encodedTopic);
+        TopicPolicies topicPolicies = getTopicPolicies(topicName).orElse(new TopicPolicies());

Review comment:
       Should we check the topic ownership check?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-728690554


   @rdhabalia Seems dup with #8555 ?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #8565: [pulsar-broker] Fix: Topic dedup policy configuration doesn't support V1 topic-name

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8565:
URL: https://github.com/apache/pulsar/pull/8565#issuecomment-731491592


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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:
users@infra.apache.org