[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #10254: [Broker] Remove possible error case for system topic name checking

[GitBox]

michaeljmarshall commented on a change in pull request #10254:
URL: https://github.com/apache/pulsar/pull/10254#discussion_r622726142



##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/systopic/SystemTopicClient.java
##########
@@ -167,8 +167,11 @@
         SystemTopicClient<T> getSystemTopic();
     }
 
+    /**
+     * A topic is a system topic if and only if its local name has the defined 
prefix.
+     */
     static boolean isSystemTopic(TopicName topicName) {
-        return 
EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getLocalName());
+        return 
topicName.getLocalName().startsWith(EventsTopicNames.SYSTEM_TOPIC_LOCAL_NAME_PREFIX);

Review comment:
       That is a great point. It does seem like there is already a bit of a 
trend where system topics begin with `__`, but you're right that it would need 
to be introduced more intentionally than this PR.
   
   I'll look at changing this PR accordingly.




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