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 2021/01/27 06:41:56 UTC

[GitHub] [pulsar] 315157973 opened a new pull request #9339: Deduplication

315157973 opened a new pull request #9339:
URL: https://github.com/apache/pulsar/pull/9339


   Master Issue: #9216
   
   ### Modifications
   1) The api name of topic-level is consistent with that of namespace-level
   2) Fix the problem that the namespace-level policy cannot be removed
   3) Fix the problem that topic-level does not take effect when multiple levels of policy are set at the same time
   4) Added applied API
   
   ### Verifying this change
   Verify that the new API is correct
   Verify that the applied API is correct
   Verify that policies of different levels exist at the same time and whether the priority is correct


----------------------------------------------------------------
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] 315157973 commented on a change in pull request #9339: Support get topic applied policy for DeduplicationStatus

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
##########
@@ -335,6 +335,16 @@ public void setSubscriptionExpirationTime(@PathParam("tenant") String tenant,
         internalSetSubscriptionExpirationTime(expirationTime);
     }
 
+    @GET
+    @Path("/{tenant}/{namespace}/deduplication")

Review comment:
       The user can get entire policies, or only a single piece of policies, there is no conflict




----------------------------------------------------------------
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] 315157973 commented on a change in pull request #9339: Support get topic applied policy for DeduplicationStatus

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
##########
@@ -335,6 +335,16 @@ public void setSubscriptionExpirationTime(@PathParam("tenant") String tenant,
         internalSetSubscriptionExpirationTime(expirationTime);
     }
 
+    @GET
+    @Path("/{tenant}/{namespace}/deduplication")

Review comment:
       The documentation we give to users does not say that every GET interface returns all the data. Although we can handle it in the SDK, but the user may directly call the API.
   So I think it is better to open the interface separately.




----------------------------------------------------------------
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 a change in pull request #9339: Support get topic applied policy for DeduplicationStatus

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
##########
@@ -335,6 +335,16 @@ public void setSubscriptionExpirationTime(@PathParam("tenant") String tenant,
         internalSetSubscriptionExpirationTime(expirationTime);
     }
 
+    @GET
+    @Path("/{tenant}/{namespace}/deduplication")

Review comment:
       We already have policy rest API. The `deduplication` is a property in the policy. Why do we need to introduce a new Rest API for this?




----------------------------------------------------------------
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] 315157973 commented on a change in pull request #9339: Support get topic applied policy for DeduplicationStatus

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
##########
@@ -335,6 +335,16 @@ public void setSubscriptionExpirationTime(@PathParam("tenant") String tenant,
         internalSetSubscriptionExpirationTime(expirationTime);
     }
 
+    @GET
+    @Path("/{tenant}/{namespace}/deduplication")

Review comment:
       We do not yet have an interface to return the entire policies, but I don't think it is a good way to return the entire policies every time. 




----------------------------------------------------------------
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] 315157973 commented on a change in pull request #9339: Support get topic applied policy for DeduplicationStatus

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
##########
@@ -335,6 +335,16 @@ public void setSubscriptionExpirationTime(@PathParam("tenant") String tenant,
         internalSetSubscriptionExpirationTime(expirationTime);
     }
 
+    @GET
+    @Path("/{tenant}/{namespace}/deduplication")

Review comment:
       The documentation we give to users does not say that every interface returns all the data. Although we can handle it in the SDK, the user may directly call the API.
   So I think it is better to open the interface separately.




----------------------------------------------------------------
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 a change in pull request #9339: Support get topic applied policy for DeduplicationStatus

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java
##########
@@ -335,6 +335,16 @@ public void setSubscriptionExpirationTime(@PathParam("tenant") String tenant,
         internalSetSubscriptionExpirationTime(expirationTime);
     }
 
+    @GET
+    @Path("/{tenant}/{namespace}/deduplication")

Review comment:
       We have the get namespace policy API: https://github.com/apache/pulsar/blob/7c6f5e2bbc8b753609b89a7589f480a57c711260/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java#L119




----------------------------------------------------------------
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 #9339: Support get topic applied policy for DeduplicationStatus

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


   @zymap Can you review it again?


----------------------------------------------------------------
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 merged pull request #9339: Support get topic applied policy for DeduplicationStatus

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9339:
URL: https://github.com/apache/pulsar/pull/9339


   


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