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/09/15 01:29:42 UTC
[GitHub] [pulsar] aloyszhang opened a new pull request #12041: make partitioned-lookup result more readable
aloyszhang opened a new pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041
Fixes #12026
### Motivation
Currently we get the broker URL for each partition of a patitioned topic, and the result displayed like
```bash
"persistent://t/ns/t-partition-0 pulsar://localhost:6650"
"persistent://t/ns/t-partition-1 pulsar://localhost:6650"
"persistent://t/ns/t-partition-2 pulsar://localhost:6650"
```
As desceibe in #12026 , if a partitioned-topic has lots of partitions(may be 1000 or more), we can't get the brokerUrl easily.
This pull request modified the result of partitiond-lookup, and return a Map<brokerUrl, List<partitioin> which is more readable from user side.
### Modifications
Modidy the result of partitioned-lookup from Map<partition, brokerUrl> to Map<brokerUrl, List<partitioin>
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change is already covered by existing tests, such as *AdminApiTest.testPulsarAdminForUriAndUrlEncoding * and *TopicOwnerTest.testLookupPartitionedTopic*.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): ( no)
- The public API: ( no)
- The schema: ( no)
- The default values of configurations: ( no)
- The wire protocol: ( no)
- The rest endpoints: ( no)
- The admin cli options: ( no)
- Anything that affects deployment: ( no)
### Documentation
no-need-doc
--
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] aloyszhang commented on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-924818972
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang closed pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang closed pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041
--
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] aloyszhang edited a comment on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang edited a comment on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-924538704
@eolivelli
Thanks for your suggestion.
How about add a new option like `--keyByBrokerUrl` to enable the new diplay for `partitioned-lookup` and default this API will work as before.
--
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] aloyszhang commented on a change in pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#discussion_r713841941
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Lookup.java
##########
@@ -57,7 +58,7 @@
* @param topic
* @return the broker URLs that serves the topic
*/
- CompletableFuture<Map<String, String>> lookupPartitionedTopicAsync(String topic);
+ CompletableFuture<Map<String, List<String>>> lookupPartitionedTopicAsync(String topic);
Review comment:
Sorry for the misunderstanding. I have modified the API implmention.
Firstly, the API will keep compatibility, `patitioned-lookup` result is still the type of `Map<String, String>`.
For the new display, add an option `--key-by-broker-url` which default is false.
If enalbe `--key-by-broker-url`, the result will be keyed by broekrUrl, and the value is the `List<String>.toString` which contains all partitions belong to the topic on this broker.
--
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] aloyszhang commented on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-927825422
@eolivelli PTAL
--
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] aloyszhang removed a comment on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang removed a comment on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-924818972
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-919645995
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-921719148
@BewareMyPower @hangc0276 PTAL
--
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] aloyszhang commented on a change in pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#discussion_r713841941
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Lookup.java
##########
@@ -57,7 +58,7 @@
* @param topic
* @return the broker URLs that serves the topic
*/
- CompletableFuture<Map<String, String>> lookupPartitionedTopicAsync(String topic);
+ CompletableFuture<Map<String, List<String>>> lookupPartitionedTopicAsync(String topic);
Review comment:
Sorry for the misunderstanding. I have modified the API implmention.
The API will keep compatibility, `patitioned-lookup` result is still the type of `Map<String, String>`.
For the new display, add an option `--key-by-broker-url` which default is false.
If enalbe `--key-by-broker-url`, the result will be keyed by broekrUrl, and the value is the `List<String>.toString` which contains all partitions belong to the topic on this broker.
--
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] aloyszhang commented on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-924538704
@eolivelli Thanks for your suggestion, how about add a new option like `-keyByBrokerUrl` to enable the new diplay for `partitioned-lookup` and default this API will work as before.
--
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] aloyszhang commented on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-924850549
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#discussion_r713841941
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Lookup.java
##########
@@ -57,7 +58,7 @@
* @param topic
* @return the broker URLs that serves the topic
*/
- CompletableFuture<Map<String, String>> lookupPartitionedTopicAsync(String topic);
+ CompletableFuture<Map<String, List<String>>> lookupPartitionedTopicAsync(String topic);
Review comment:
@eolivelli Sorry for the misunderstanding. I have modified the API implmention.
Firstly, the API will keep compatibility, `patitioned-lookup` result is still the type of `Map<String, String>`.
For the new display, add an option `--key-by-broker-url` which default is false.
If enalbe `--key-by-broker-url`, the result will be keyed by broekrUrl, and the value is the `List<String>.toString` which contains all partitions belong to the topic on this broker.
--
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] aloyszhang edited a comment on pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
aloyszhang edited a comment on pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#issuecomment-924538704
@eolivelli Thanks for your suggestion, how about add a new option like `--keyByBrokerUrl` to enable the new diplay for `partitioned-lookup` and default this API will work as before.
--
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] eolivelli commented on a change in pull request #12041: make partitioned-lookup result more readable
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12041:
URL: https://github.com/apache/pulsar/pull/12041#discussion_r713601391
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Lookup.java
##########
@@ -57,7 +58,7 @@
* @param topic
* @return the broker URLs that serves the topic
*/
- CompletableFuture<Map<String, String>> lookupPartitionedTopicAsync(String topic);
+ CompletableFuture<Map<String, List<String>>> lookupPartitionedTopicAsync(String topic);
Review comment:
To be clear: this is the breaking API change that I am referring to
--
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