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 2021/02/26 07:40:14 UTC
[GitHub] [pulsar] Renkai opened a new pull request #9734: Add mirror method for getPersistenceNamingEncoding
Renkai opened a new pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734
This method will be used for where we need to get Schema info from the persistent name in some plugins.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] zymap commented on a change in pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
zymap commented on a change in pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#discussion_r583568333
##########
File path: pulsar-common/src/test/java/org/apache/pulsar/common/naming/TopicNameTest.java
##########
@@ -162,6 +161,9 @@ public void topic() {
assertEquals(TopicName.get("persistent://tenant/cluster/namespace/topic")
.getPersistenceNamingEncoding(), "tenant/cluster/namespace/persistent/topic");
+ assertEquals(TopicName.fromPersistenceNamingEncoding("tenant/cluster/namespace/persistent/topic"),
Review comment:
You may need to add a case for using the `getPersistenceNamingEncoding` returned value to parse to ensure the encoded value can be parsed successfully.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#issuecomment-1058892891
The pr had no activity for 30 days, mark with Stale label.
--
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] Renkai commented on pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
Renkai commented on pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#issuecomment-786541742
/pulsarbot run-failure-checks
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] Renkai commented on a change in pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
Renkai commented on a change in pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#discussion_r583714245
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -320,6 +322,33 @@ public String getPersistenceNamingEncoding() {
}
}
+ public static TopicName fromPersistenceNamingEncoding(String name) throws Exception {
Review comment:
It's not for "one plugin", but maight be used for various plugins which may need to parse managed ledger name
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] zymap commented on a change in pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
zymap commented on a change in pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#discussion_r583551141
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -320,6 +322,33 @@ public String getPersistenceNamingEncoding() {
}
}
+ public static TopicName fromPersistenceNamingEncoding(String name) throws Exception {
Review comment:
I'm not sure what is this used for? Looks like we can parse it in the plugin and avoid introducing it into Pulsar?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#issuecomment-1058893096
@Renkai:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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] 315157973 commented on a change in pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#discussion_r584410222
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -320,6 +322,33 @@ public String getPersistenceNamingEncoding() {
}
}
+ public static TopicName fromPersistenceNamingEncoding(String name) throws Exception {
+ if (name == null) {
+ return null;
+ }
+ final String[] arr = name.split("/");
+
+ if (arr.length == 4) {
+ String tenant = arr[0];
+ String namespacePortion = arr[1];
+ String domain = arr[2];
+ String encodedLocalName = arr[3];
+ final String decodedName = Codec.decode(encodedLocalName);
+ return TopicName.get(domain, tenant, namespacePortion, decodedName);
+ } else if (arr.length == 5) {
+ String tenant = arr[0];
+ String cluster = arr[1];
+ String namespacePortion = arr[2];
+ String domain = arr[3];
+ String encodedLocalName = arr[4];
+ final String decodedName = Codec.decode(encodedLocalName);
+ return TopicName.get(domain, tenant, cluster, namespacePortion, decodedName);
+ } else {
+ log.error("arr.length = {}, arr = {}", arr.length, Stream.of(arr).collect(Collectors.toList()));
+ throw new Exception("not valid name: " + name);
Review comment:
No unit test coverage for this branch
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a change in pull request #9734: Add mirror method for getPersistenceNamingEncoding
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #9734:
URL: https://github.com/apache/pulsar/pull/9734#discussion_r584390182
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -320,6 +322,33 @@ public String getPersistenceNamingEncoding() {
}
}
+ public static TopicName fromPersistenceNamingEncoding(String name) throws Exception {
+ if (name == null) {
+ return null;
+ }
+ final String[] arr = name.split("/");
+
+ if (arr.length == 4) {
+ String tenant = arr[0];
+ String namespacePortion = arr[1];
+ String domain = arr[2];
+ String encodedLocalName = arr[3];
+ final String decodedName = Codec.decode(encodedLocalName);
Review comment:
Looks this case does not cover by any test? Could you please also add a test for this case?
----------------------------------------------------------------
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:
users@infra.apache.org