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/08/14 04:26:28 UTC

[GitHub] [pulsar] zhanghaou opened a new pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

zhanghaou opened a new pull request #7817:
URL: https://github.com/apache/pulsar/pull/7817


   Link [https://github.com/apache/pulsar/issues/7757](https://github.com/apache/pulsar/issues/7757) and master issue [https://github.com/apache/pulsar/issues/2688](https://github.com/apache/pulsar/issues/2688)
   
   ### Motivation
   
   Support set/get/remove persistence  policies on topic level.
   
   ### Verifying this change
   new unit test added.
   
   


----------------------------------------------------------------
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] zhanghaou commented on a change in pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -2294,6 +2295,48 @@ protected void internalGetRetention(AsyncResponse asyncResponse){
         return pulsar().getTopicPoliciesService().updateTopicPoliciesAsync(topicName, topicPolicies.get());
     }
 
+    protected void internalGetPersistence(AsyncResponse asyncResponse){
+        validateAdminAccessForTenant(namespaceName.getTenant());
+        validatePoliciesReadOnlyAccess();
+        if (topicName.isGlobal()) {
+            validateGlobalNamespaceOwnership(namespaceName);
+        }
+        Optional<PersistencePolicies> retention = getTopicPolicies(topicName)
+                .map(TopicPolicies::getPersistence);
+        if (!retention.isPresent()) {
+            asyncResponse.resume(Response.noContent().build());
+        }else {
+            asyncResponse.resume(retention.get());
+        }
+    }

Review comment:
       It is same to internalGetRetention(), maybe we can modify it all at once.




----------------------------------------------------------------
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] zhanghaou commented on pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

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


   > The change looks do not handle the topic creation according to the topic policy if the topic level persistent policy exists. You can refer to `createPersistentTopic` method in the BrokerService.java
   
   Done.


----------------------------------------------------------------
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] zhanghaou commented on pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

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


   /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] zhanghaou removed a comment on pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

Posted by GitBox <gi...@apache.org>.
zhanghaou removed a comment on pull request #7817:
URL: https://github.com/apache/pulsar/pull/7817#issuecomment-673915790


   /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] zhanghaou commented on pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

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


   /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] zhanghaou commented on pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

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


   /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] jianyun8023 commented on a change in pull request #7817: [ISSUE 7757] Support Persistence Policies on topic level

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -2294,6 +2295,48 @@ protected void internalGetRetention(AsyncResponse asyncResponse){
         return pulsar().getTopicPoliciesService().updateTopicPoliciesAsync(topicName, topicPolicies.get());
     }
 
+    protected void internalGetPersistence(AsyncResponse asyncResponse){
+        validateAdminAccessForTenant(namespaceName.getTenant());
+        validatePoliciesReadOnlyAccess();
+        if (topicName.isGlobal()) {
+            validateGlobalNamespaceOwnership(namespaceName);
+        }
+        Optional<PersistencePolicies> retention = getTopicPolicies(topicName)
+                .map(TopicPolicies::getPersistence);
+        if (!retention.isPresent()) {
+            asyncResponse.resume(Response.noContent().build());
+        }else {
+            asyncResponse.resume(retention.get());
+        }
+    }

Review comment:
       minor: there is no need to use `asyncResponse`




----------------------------------------------------------------
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 #7817: [ISSUE 7757] Support Persistence Policies on topic level

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


   @jianyun8023 Please help review this PR.


----------------------------------------------------------------
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 #7817: [ISSUE 7757] Support Persistence Policies on topic level

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


   


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