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/08/09 05:16:51 UTC

[GitHub] [pulsar] Technoboy- opened a new pull request, #17006: [improve][admin] Not allow to terminate system topic.

Technoboy- opened a new pull request, #17006:
URL: https://github.com/apache/pulsar/pull/17006

   Fixes #15891
   
   Master Issue: #15891
   
   ### Motivation
   
   Pulsar-manager could see all the topics including the system topic and can terminate them.  If terminates system topic by mistake, there would cause many serious problems.  So it's better not allow to terminate the system topic.
   
   ### Documentation
   
   - [x] `doc-not-needed` 
   (Please explain why)
   


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


[GitHub] [pulsar] Technoboy- commented on pull request #17006: [improve][admin] Not allow to terminate system topic.

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on PR #17006:
URL: https://github.com/apache/pulsar/pull/17006#issuecomment-1209275508

   > What about terminating the partitioned topic?
   
   Not allowed. The logic is here 
   https://github.com/apache/pulsar/blob/e1c0ad395c19c99369e733f44a2426de5c465787/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L3686-L3691


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


[GitHub] [pulsar] Technoboy- commented on a diff in pull request #17006: [improve][admin] Not allow to terminate system topic.

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #17006:
URL: https://github.com/apache/pulsar/pull/17006#discussion_r942024560


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java:
##########
@@ -2533,4 +2533,21 @@ public void testGetNamespaceTopicList() throws Exception {
                 ListNamespaceTopicsOptions.builder().mode(Mode.NON_PERSISTENT).build());
         Assert.assertTrue(notPersistentTopics.contains(nonPersistentTopic));
     }
+
+    @Test
+    private void testTerminateSystemTopic() throws Exception {
+        final String topic = "persistent://prop-xyz/ns1/testTerminateSystemTopic";
+        admin.topics().createNonPartitionedTopic(topic);
+        final String eventTopic = "persistent://prop-xyz/ns1/__change_events";
+        admin.topicPolicies().setMaxConsumers(topic, 2);
+        Awaitility.await().untilAsserted(() -> {
+            Assert.assertEquals(admin.topicPolicies().getMaxConsumers(topic), Integer.valueOf(2));
+        });
+        try {

Review Comment:
   applied.



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


[GitHub] [pulsar] nodece commented on a diff in pull request #17006: [improve][admin] Not allow to terminate system topic.

Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #17006:
URL: https://github.com/apache/pulsar/pull/17006#discussion_r941435814


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java:
##########
@@ -2533,4 +2533,21 @@ public void testGetNamespaceTopicList() throws Exception {
                 ListNamespaceTopicsOptions.builder().mode(Mode.NON_PERSISTENT).build());
         Assert.assertTrue(notPersistentTopics.contains(nonPersistentTopic));
     }
+
+    @Test
+    private void testTerminateSystemTopic() throws Exception {
+        final String topic = "persistent://prop-xyz/ns1/testTerminateSystemTopic";
+        admin.topics().createNonPartitionedTopic(topic);
+        final String eventTopic = "persistent://prop-xyz/ns1/__change_events";
+        admin.topicPolicies().setMaxConsumers(topic, 2);
+        Awaitility.await().untilAsserted(() -> {
+            Assert.assertEquals(admin.topicPolicies().getMaxConsumers(topic), Integer.valueOf(2));
+        });
+        try {

Review Comment:
   ```suggestion
           PulsarAdminException ex = expectThrows(PulsarAdminException.class, () -> admin.topics().terminateTopic(eventTopic));
           assertTrue(ex instanceof PulsarAdminException.NotAllowedException);
   ```



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


[GitHub] [pulsar] github-actions[bot] commented on pull request #17006: [improve][admin] Not allow to terminate system topic.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17006:
URL: https://github.com/apache/pulsar/pull/17006#issuecomment-1208932105

   @Technoboy- Please provide a correct documentation label for your PR.
   Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).


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


[GitHub] [pulsar] Technoboy- merged pull request #17006: [improve][admin] Not allow to terminate system topic.

Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #17006:
URL: https://github.com/apache/pulsar/pull/17006


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