[GitHub] [pulsar] shibd commented on a diff in pull request #19667: [fix][broker] Filter system topic when getting topic list by binary proto.

["shibd (via GitHub)" <gi...@apache.org>]

shibd commented on code in PR #19667:
URL: https://github.com/apache/pulsar/pull/19667#discussion_r1123156520


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java:
##########
@@ -2182,8 +2182,8 @@ protected void handleGetTopicsOfNamespace(CommandGetTopicsOfNamespace commandGet
                     getBrokerService().pulsar().getNamespaceService().getListOfTopics(namespaceName, mode)
                         .thenAccept(topics -> {
                             boolean filterTopics = false;
-                            // filter transaction internal topic
-                            List<String> filteredTopics = TopicList.filterTransactionInternalName(topics);
+                            // filter system topic
+                            List<String> filteredTopics = TopicList.filterSystemTopic(topics);

Review Comment:
   I think we should discuss clearly first: What system topic should be exposed to users?
   
   These system themes are all used internally(`__change_event` and transaction-related topics), and I don't think they should be exposed to users.
   
   So, Maybe this PR isn't a breaking change, It fixes compatibility issues caused by PRs like #16042
   
   



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