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 2022/06/06 05:04:23 UTC
[GitHub] [pulsar] Technoboy- opened a new pull request, #15944: [feature][admin] Support to get non-partitioned topic properties.
Technoboy- opened a new pull request, #15944:
URL: https://github.com/apache/pulsar/pull/15944
### Motivation
As https://github.com/apache/pulsar/pull/12818 has supported creating topics with metadata, this patch is adding a `get` API to support getting non-partitioned topic properties.
### Documentation
- [x] `doc-not-needed`
(Please explain why)
--
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] mattisonchao commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r890773773
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
+ .thenCompose(metadata -> {
+ if (metadata.partitions == 0 || topicName.isPartitioned()) {
Review Comment:
When `topicName.isPartitioned()` is true, the `metadata.partitions` may always equals to 0 here.
--
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] mattisonchao commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r890775769
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
Review Comment:
If the topic name is partitioned we may don't need to `fetchPartitionedTopicMetadataAsync`.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
+ .thenCompose(metadata -> {
+ if (metadata.partitions == 0 || topicName.isPartitioned()) {
Review Comment:
When `topicName.isPartitioned()` is true, looks like `metadata.partitions` is always equals to 0;
--
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] heesung-sn commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r1199274764
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,34 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOwnershipAsync(topicName, authoritative)
+ .thenCompose(__ -> validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA))
+ .thenCompose(__ -> {
+ if (topicName.isPartitioned()) {
+ return getPropertiesAsync();
+ }
+ return pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName)
+ .thenCompose(metadata -> {
+ if (metadata.partitions == 0) {
+ return getPropertiesAsync();
+ }
+ return CompletableFuture.completedFuture(metadata.properties);
+ });
+ });
+ }
+
+ private CompletableFuture<Map<String, String>> getPropertiesAsync() {
+ return pulsar().getBrokerService().getTopicIfExists(topicName.toString())
+ .thenApply(opt -> {
+ if (!opt.isPresent()) {
+ throw new RestException(Status.NOT_FOUND,
+ getTopicNotFoundErrorMessage(topicName.toString()));
+ }
+ return ((PersistentTopic) opt.get()).getManagedLedger().getProperties();
Review Comment:
https://github.com/apache/pulsar/pull/12818/files introduced ManagedLedgerConfig.properties
https://github.com/apache/pulsar/pull/12818/files#diff-c917471bc697b80ff5e8eb5de212597c527e756ea49694c7fbbabacbf2b9752cR76.
However, we are returning ManagedLedgerImpl.propertiesMap here.
Why do we have two properties in two different places?
--
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] codelipenghui merged pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #15944:
URL: https://github.com/apache/pulsar/pull/15944
--
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] Technoboy- commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r890821541
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
Review Comment:
yes, fixed.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
+ .thenCompose(metadata -> {
+ if (metadata.partitions == 0 || topicName.isPartitioned()) {
Review Comment:
yes，fixed.
--
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] mattisonchao commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r890771284
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
Review Comment:
Question: What happens when the user passes in a partitioned topic?
--
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] mattisonchao commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r890771284
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
Review Comment:
Question: What happens when the user passes in a partitioned topic?
--
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] mattisonchao commented on a diff in pull request #15944: [feature][admin] Support to get topic properties.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15944:
URL: https://github.com/apache/pulsar/pull/15944#discussion_r890771284
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -565,6 +565,25 @@ protected CompletableFuture<PartitionedTopicMetadata> internalGetPartitionedMeta
});
}
+ protected CompletableFuture<Map<String, String>> internalGetPropertiesAsync(boolean authoritative) {
+ return validateTopicOperationAsync(topicName, TopicOperation.GET_METADATA)
+ .thenCompose(__ -> pulsar().getBrokerService().fetchPartitionedTopicMetadataAsync(topicName))
Review Comment:
What happens when the user passes in a partitioned topic?
--
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