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/08/31 15:16:30 UTC
[GitHub] [pulsar] Pomelongan opened a new pull request, #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Pomelongan opened a new pull request, #17378:
URL: https://github.com/apache/pulsar/pull/17378
### Motivation
When the org.apache.pulsar.broker.admin.AdminResource#tryCreatePartitionAsync method is called, the value of reusefuture passed in is null, which is a bit redundant and can be directly removed
### Modifications
*Describe the modifications you've done.*
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change is a trivial rework / code cleanup without any test coverage.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [x] `doc-not-needed`
--
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] Pomelongan commented on pull request #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Pomelongan commented on PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#issuecomment-1234368297
/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] Jason918 commented on a diff in pull request #17378: [cleanup][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#discussion_r962248310
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java:
##########
@@ -157,13 +157,13 @@ protected CompletableFuture<Void> tryCreatePartitionsAsync(int numPartitions) {
}
List<CompletableFuture<Void>> futures = new ArrayList<>(numPartitions);
for (int i = 0; i < numPartitions; i++) {
- futures.add(tryCreatePartitionAsync(i, null));
+ futures.add(tryCreatePartitionAsync(i));
}
return FutureUtil.waitForAll(futures);
}
- private CompletableFuture<Void> tryCreatePartitionAsync(final int partition, CompletableFuture<Void> reuseFuture) {
- CompletableFuture<Void> result = reuseFuture == null ? new CompletableFuture<>() : reuseFuture;
Review Comment:
> I don't suggest we change signature of method for the might extension in the future
@gaozhangmin
I don't see any necessary to keep this, at least in the near future. Let's make the code cleaner.
--
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 commented on pull request #17378: [cleanup][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#issuecomment-1236257943
/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] Pomelongan commented on a diff in pull request #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Pomelongan commented on code in PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#discussion_r960730516
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java:
##########
@@ -157,13 +157,13 @@ protected CompletableFuture<Void> tryCreatePartitionsAsync(int numPartitions) {
}
List<CompletableFuture<Void>> futures = new ArrayList<>(numPartitions);
for (int i = 0; i < numPartitions; i++) {
- futures.add(tryCreatePartitionAsync(i, null));
+ futures.add(tryCreatePartitionAsync(i));
}
return FutureUtil.waitForAll(futures);
}
- private CompletableFuture<Void> tryCreatePartitionAsync(final int partition, CompletableFuture<Void> reuseFuture) {
- CompletableFuture<Void> result = reuseFuture == null ? new CompletableFuture<>() : reuseFuture;
Review Comment:
IMHO：
The parameter reusefuture was added for a function in the PR #6478, but it was later removed in the PR #9485, so it is reasonable to restore the parameter to its original state.
--
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] Pomelongan commented on pull request #17378: [cleanup][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Pomelongan commented on PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#issuecomment-1238868458
/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] gaozhangmin commented on a diff in pull request #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#discussion_r959785150
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java:
##########
@@ -157,13 +157,13 @@ protected CompletableFuture<Void> tryCreatePartitionsAsync(int numPartitions) {
}
List<CompletableFuture<Void>> futures = new ArrayList<>(numPartitions);
for (int i = 0; i < numPartitions; i++) {
- futures.add(tryCreatePartitionAsync(i, null));
+ futures.add(tryCreatePartitionAsync(i));
}
return FutureUtil.waitForAll(futures);
}
- private CompletableFuture<Void> tryCreatePartitionAsync(final int partition, CompletableFuture<Void> reuseFuture) {
- CompletableFuture<Void> result = reuseFuture == null ? new CompletableFuture<>() : reuseFuture;
Review Comment:
I don't suggest we change signature of method for the might extension in the future
--
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] AnonHxy commented on pull request #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#issuecomment-1233138911
LGTM
--
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] Pomelongan commented on pull request #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Pomelongan commented on PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#issuecomment-1234997923
/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] Pomelongan commented on a diff in pull request #17378: [improve][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
Pomelongan commented on code in PR #17378:
URL: https://github.com/apache/pulsar/pull/17378#discussion_r960730516
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java:
##########
@@ -157,13 +157,13 @@ protected CompletableFuture<Void> tryCreatePartitionsAsync(int numPartitions) {
}
List<CompletableFuture<Void>> futures = new ArrayList<>(numPartitions);
for (int i = 0; i < numPartitions; i++) {
- futures.add(tryCreatePartitionAsync(i, null));
+ futures.add(tryCreatePartitionAsync(i));
}
return FutureUtil.waitForAll(futures);
}
- private CompletableFuture<Void> tryCreatePartitionAsync(final int partition, CompletableFuture<Void> reuseFuture) {
- CompletableFuture<Void> result = reuseFuture == null ? new CompletableFuture<>() : reuseFuture;
Review Comment:
@gaozhangmin
IMHO：
The parameter reusefuture was added for a function in the PR #6478, but it was later removed in the PR #9485, so it is reasonable to restore the parameter to its original state.
--
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 merged pull request #17378: [cleanup][broker] remove unnecessary parameters(reusefuture) and related logic
Posted by GitBox <gi...@apache.org>.
gaozhangmin merged PR #17378:
URL: https://github.com/apache/pulsar/pull/17378
--
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