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/04/23 08:41:50 UTC
[GitHub] [pulsar] baomingyu opened a new pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
baomingyu opened a new pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339
Fixes #10338
Master Issue: #10338
--
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] lhotari commented on pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#issuecomment-826349902
/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] linlinnn commented on a change in pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#discussion_r619060925
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,19 +1515,21 @@ public void removeBacklogQuota(@Suspended final AsyncResponse asyncResponse,
public Integer getMessageTTL(@PathParam("tenant") String tenant,
@PathParam("namespace") String namespace,
@PathParam("topic") @Encoded String encodedTopic,
- @QueryParam("applied") boolean applied) {
+ @DefaultValue("false")@QueryParam("applied") boolean applied) {
Review comment:
maybe you want change like this:
`@DefaultValue("false") @QueryParam("applied") Boolean applied`
--
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] linlinnn commented on a change in pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#discussion_r619060925
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,19 +1515,21 @@ public void removeBacklogQuota(@Suspended final AsyncResponse asyncResponse,
public Integer getMessageTTL(@PathParam("tenant") String tenant,
@PathParam("namespace") String namespace,
@PathParam("topic") @Encoded String encodedTopic,
- @QueryParam("applied") boolean applied) {
+ @DefaultValue("false")@QueryParam("applied") boolean applied) {
Review comment:
maybe you want change like this:
`@DefaultValue("false") @QueryParam("applied") Boolean applied`
then the below
`if (Boolean.TRUE.equals(applied))`
keep others
--
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 #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#issuecomment-1058891137
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] linlinnn commented on a change in pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#discussion_r619060925
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,19 +1515,21 @@ public void removeBacklogQuota(@Suspended final AsyncResponse asyncResponse,
public Integer getMessageTTL(@PathParam("tenant") String tenant,
@PathParam("namespace") String namespace,
@PathParam("topic") @Encoded String encodedTopic,
- @QueryParam("applied") boolean applied) {
+ @DefaultValue("false")@QueryParam("applied") boolean applied) {
Review comment:
maybe you want change like this:
@DefaultValue("false") @QueryParam("applied") Boolean applied
--
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] MarvinCai commented on pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#issuecomment-826023351
> Could you show us the logs of NPE? I didn't see where there will be NPE
+1, orElseGet() should cover the null case, if getTopicPolicies() return null when topic doesn't exist it'll just treated as no value present
--
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] linlinnn commented on a change in pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#discussion_r619055288
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,19 +1515,21 @@ public void removeBacklogQuota(@Suspended final AsyncResponse asyncResponse,
public Integer getMessageTTL(@PathParam("tenant") String tenant,
@PathParam("namespace") String namespace,
@PathParam("topic") @Encoded String encodedTopic,
- @QueryParam("applied") boolean applied) {
+ @DefaultValue("false")@QueryParam("applied") boolean applied) {
validateTopicName(tenant, namespace, encodedTopic);
preValidation();
- return getTopicPolicies(topicName)
- .map(TopicPolicies::getMessageTTLInSeconds)
- .orElseGet(() -> {
- if (applied) {
- Integer otherLevelTTL = getNamespacePolicies(namespaceName).message_ttl_in_seconds;
- return otherLevelTTL == null ? pulsar().getConfiguration().getTtlDurationDefaultInSeconds()
- : otherLevelTTL;
- }
- return null;
- });
+ Optional<Integer> messageTTL = getTopicPolicies(topicName)
Review comment:
I'm not sure where the difference after this change
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1515,19 +1515,21 @@ public void removeBacklogQuota(@Suspended final AsyncResponse asyncResponse,
public Integer getMessageTTL(@PathParam("tenant") String tenant,
@PathParam("namespace") String namespace,
@PathParam("topic") @Encoded String encodedTopic,
- @QueryParam("applied") boolean applied) {
+ @DefaultValue("false")@QueryParam("applied") boolean applied) {
Review comment:
The primitive type `boolean`'s default value is false.
--
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 #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#issuecomment-1058891367
@baomingyu: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 pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#issuecomment-825537116
Could you show us the logs of NPE? I didn't see where there will be NPE
--
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] lhotari commented on pull request #10339: fix 10338 PersistentTopics#getMessageTTL maybe throw nullpointException
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #10339:
URL: https://github.com/apache/pulsar/pull/10339#issuecomment-826349902
/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