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/04/11 11:52:48 UTC
[GitHub] [pulsar] gaozhangmin opened a new pull request, #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
gaozhangmin opened a new pull request, #15120:
URL: https://github.com/apache/pulsar/pull/15120
### Motivation
eg, method `internalGetSubscriptions` invokes `validateTopicOwnershipAsync` twice, because method `internalGetSubscriptionsForNonPartitionedTopic` also invoked `validateTopicOwnershipAsync`.
### Modifications
Removed duplicate `validateTopicOwnershipAsync`
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `no-need-doc`
(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] github-actions[bot] commented on pull request #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#issuecomment-1094975032
@gaozhangmin:Thanks for providing doc info!
--
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] gaozhangmin commented on a diff in pull request #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#discussion_r852006484
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -1725,11 +1726,7 @@ protected void internalSkipAllMessages(AsyncResponse asyncResponse, String subNa
private CompletableFuture<Void> internalSkipAllMessagesForNonPartitionedTopicAsync(AsyncResponse asyncResponse,
String subName,
boolean authoritative) {
- return validateTopicOwnershipAsync(topicName, authoritative)
- .thenCompose(__ ->
- validateTopicOperationAsync(topicName, TopicOperation.SKIP, subName))
- .thenCompose(__ ->
Review Comment:
Call these two method outside of this method.
--
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] Jason918 merged pull request #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
Jason918 merged PR #15120:
URL: https://github.com/apache/pulsar/pull/15120
--
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 #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#discussion_r852003864
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -1725,11 +1726,7 @@ protected void internalSkipAllMessages(AsyncResponse asyncResponse, String subNa
private CompletableFuture<Void> internalSkipAllMessagesForNonPartitionedTopicAsync(AsyncResponse asyncResponse,
String subName,
boolean authoritative) {
- return validateTopicOwnershipAsync(topicName, authoritative)
- .thenCompose(__ ->
- validateTopicOperationAsync(topicName, TopicOperation.SKIP, subName))
- .thenCompose(__ ->
Review Comment:
Why remove `validateTopicOperationAsync`
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -1147,10 +1150,8 @@ private void resumeAsyncResponse(AsyncResponse asyncResponse, Set<String> subscr
});
}
- private void internalGetSubscriptionsForNonPartitionedTopic(AsyncResponse asyncResponse, boolean authoritative) {
- validateTopicOwnershipAsync(topicName, authoritative)
- .thenCompose(__ -> validateTopicOperationAsync(topicName, TopicOperation.GET_SUBSCRIPTIONS))
- .thenCompose(__ -> getTopicReferenceAsync(topicName))
Review Comment:
Why remove `validateTopicOperationAsync`
--
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] gaozhangmin commented on pull request #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#issuecomment-1095923801
/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] github-actions[bot] commented on pull request #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#issuecomment-1094970082
@gaozhangmin: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] gaozhangmin commented on pull request #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#issuecomment-1099982372
@Technoboy- PTAL Thx.
--
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 #15120: [improve][Rest api]Remove duplication validateTopicOwnershipAsync
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15120:
URL: https://github.com/apache/pulsar/pull/15120#discussion_r852003999
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -1080,7 +1082,7 @@ protected void internalGetSubscriptions(AsyncResponse asyncResponse, boolean aut
throw new RestException(e);
}
});
- }).thenAccept(__ -> resumeAsyncResponse(asyncResponse,
+ }).thenAccept(unused3 -> resumeAsyncResponse(asyncResponse,
Review Comment:
This seems ok.
--
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