michaeljmarshall commented on a change in pull request #10683:
URL: https://github.com/apache/pulsar/pull/10683#discussion_r637683306
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -2555,36 +2555,30 @@ private boolean isSystemTopic(String topic) {
}
/**
- * Get {@link TopicPolicies} for this topic.
+ * Get {@link TopicPolicies} for the parameterized topic.
* @param topicName
* @return TopicPolicies is exist else return null.
*/
public TopicPolicies getTopicPolicies(TopicName topicName) {
+ if (!pulsar().getConfig().isTopicLevelPoliciesEnabled()) {
+ return null;
+ }
TopicName cloneTopicName = topicName;
if (topicName.isPartitioned()) {
cloneTopicName =
TopicName.get(topicName.getPartitionedTopicName());
}
try {
- checkTopicLevelPolicyEnable();
return
pulsar.getTopicPoliciesService().getTopicPolicies(cloneTopicName);
} catch (BrokerServiceException.TopicPoliciesCacheNotInitException e) {
log.debug("Topic {} policies have not been initialized yet.",
topicName.getPartitionedTopicName());
return null;
- } catch (RestException | NullPointerException e) {
+ } catch (NullPointerException e) {
Review comment:
I agree that this is code smell. I don't see any way for this `NPE` to
happen (even before this PR's changes), and now that we're checking the
configuration before getting the topic policy, I see no need for it. I'll
remove it.
--
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:
[email protected]
