pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] rdhabalia commented on a change in pull request #2828: Support topic name with / in name
Date Wed, 24 Oct 2018 01:14:36 GMT
rdhabalia commented on a change in pull request #2828: Support topic name with / in name
URL: https://github.com/apache/pulsar/pull/2828#discussion_r227615502
 
 

 ##########
 File path: pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
 ##########
 @@ -139,6 +139,15 @@ private TopicName(String completeTopicName) {
 
 
             parts = Splitter.on("/").limit(4).splitToList(rest);
+            if (parts.size() == 4) {
+                try {
+                    NamedEntity.checkName(parts.get(2));
+                } catch (IllegalArgumentException ie) {
+                    // It happens when topic-local-name has "/" in the name and cluster doesn't
present into the
+                    // topic-name
+                    parts = Splitter.on("/").limit(3).splitToList(rest);
+                }
+            }
 
 Review comment:
   actually, that is true. my test-case had multiple special chars so, namespace validation
was failing and it was falling back to this logic and things worked.
   
   `parts` size can be 4 in below cases:
   1. topic-name w/o special-char and topic with cluster: `tenant/cluster/ns/$topic (topic=my-topic)`
   In this case, it works fine.
   
   2. topic-name with special char and topic with cluster: `tenant/cluster/ns/$topic (topic=my/topic)`
   in this case also it works fine. `cluster=cluster`,`namespace=ns`, and `topic=my/topic`
   
   3. topic-name with special char and topic without cluster: `tenant/ns/$topic (topic=my/topic)`
   In this case: it fails: `cluster=ns`,`namespace=my`, and `topic=topic`
   
   So, probably, we might need explicit flag which tells TopicName that `cluster` is not provided
so, TopicName parsing should only parse 3 fields : tenant, ns and topic-name.? 
   ```
   TopicName(String topic, boolean hasCluster=false) {..}
   ```
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

Mime
View raw message