-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ISSUE 7758] Support set Max Producer on topic level. #7914
Conversation
# Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
link #7839 PTAL @zhaijack @codelipenghui |
@jianyun8023 please help review this PR |
# Conflicts: # pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesDisableTest.java # pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/Topics.java # pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java # pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
/pulsarbot run-failure-checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
protected void internalGetMaxProducers(AsyncResponse asyncResponse) { | ||
validateAdminAccessForTenant(namespaceName.getTenant()); | ||
validatePoliciesReadOnlyAccess(); | ||
if (topicName.isGlobal()) { | ||
validateGlobalNamespaceOwnership(namespaceName); | ||
} | ||
checkTopicLevelPolicyEnable(); | ||
Optional<Integer> maxProducers = getTopicPolicies(topicName) | ||
.map(TopicPolicies::getMaxProducerPerTopic); | ||
if (!maxProducers.isPresent()) { | ||
asyncResponse.resume(Response.noContent().build()); | ||
} else { | ||
asyncResponse.resume(maxProducers.get()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please return CompletableFuture< Integer >
instead of passing in AsyncResponse
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or directly return Integer
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems some methods similar to this also need to modify, can we modify it all?
# Conflicts: # pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesDisableTest.java # pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/Topics.java # pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java # pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
Link [apache#7758 and master issue [apache#2688 ### Motivation Support set/get/remove maxProducers on a topic level.
Link [apache#7758 and master issue [apache#2688 ### Motivation Support set/get/remove maxProducers on a topic level.
Link [apache#7758 and master issue [apache#2688 ### Motivation Support set/get/remove maxProducers on a topic level.
Link #7758 and master issue #2688
Motivation
Support set/get/remove maxProducers on a topic level.
Verifying this change
new unit test added.