poorbarcode commented on code in PR #19841:
URL: https://github.com/apache/pulsar/pull/19841#discussion_r1160405999


##########
pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java:
##########
@@ -231,7 +231,7 @@ public String getEncodedLocalName() {
     }
 
     public TopicName getPartition(int index) {
-        if (index == -1 || this.toString().contains(PARTITIONED_TOPIC_SUFFIX)) 
{
+        if (index == -1 || this.toString().endsWith(PARTITIONED_TOPIC_SUFFIX + 
index)) {

Review Comment:
   > What I thought was the PR intent to allow the partitioned name can contain 
-partition- keyword to resolve the issue. But it will introduce a new 
unreasonable rule to the partitioned topic.
   
   I think there are three things:
   - Our rule for a partition name and how to check which index it is: end with 
`-partition-x` or contain `-partition-x`
     - The current logic is incorrect, and current PR can fix it. 
   - Whether to allow to create a topic whose name contains `-paritition-`
     - the behavior is denied for Admin API and is allowed for client(such as 
create by a cmd-subscribe), you can see the tests 
`testInfiniteHttpCallGetSubscriptions2`, 
`testInfiniteHttpCallGetSubscriptions3` in current PR
   - Whether only create non-partitioned topics for the Dead Letter or Retry 
Letter
     - [PIP-263: Just auto-create no-partitioned DLQ And Prevent auto-create a 
DLQ for a DLQ]( https://github.com/apache/pulsar/issues/20033) try to do 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.

To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to