Re: [PR] MINOR: enhance kafka-reassign-partitions command output [kafka]
showuon merged PR #15610: URL: https://github.com/apache/kafka/pull/15610 -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: enhance kafka-reassign-partitions command output [kafka]
showuon commented on PR #15610: URL: https://github.com/apache/kafka/pull/15610#issuecomment-2031206526 Failed tests are unrelated. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: enhance kafka-reassign-partitions command output [kafka]
showuon commented on PR #15610: URL: https://github.com/apache/kafka/pull/15610#issuecomment-2029071458 @AndrewJSchofield , Do you have any other comments? I'm going to merge this PR this week if no other comments. 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: enhance kafka-reassign-partitions command output [kafka]
KevinZTW commented on code in PR #15610:
URL: https://github.com/apache/kafka/pull/15610#discussion_r1542216359
##
tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java:
##
@@ -806,12 +806,10 @@ private static void executeMoves(Admin adminClient,
do {
Set completed =
alterReplicaLogDirs(adminClient, pendingReplicas);
if (!completed.isEmpty()) {
-System.out.printf("Successfully started log directory move%s
for: %s%n",
-completed.size() == 1 ? "" : "s",
-completed.stream()
-
.sorted(ReassignPartitionsCommand::compareTopicPartitionReplicas)
-.map(Object::toString)
-.collect(Collectors.joining(",")));
+
completed.stream().sorted(ReassignPartitionsCommand::compareTopicPartitionReplicas).forEach(replica
-> {
+System.out.printf("Successfully started moving log
directory to %s for replica %s-%s with broker id: %s %n",
Review Comment:
Thank you for the suggestion! let me revise this
##
tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java:
##
@@ -1485,6 +1483,7 @@ static Set
alterReplicaLogDirs(Admin adminClient,
for (Entry> e :
values.entrySet()) {
TopicPartitionReplica replica = e.getKey();
KafkaFuture future = e.getValue();
+
Review Comment:
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINOR: enhance kafka-reassign-partitions command output [kafka]
showuon commented on code in PR #15610:
URL: https://github.com/apache/kafka/pull/15610#discussion_r1542188367
##
tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java:
##
@@ -1485,6 +1483,7 @@ static Set
alterReplicaLogDirs(Admin adminClient,
for (Entry> e :
values.entrySet()) {
TopicPartitionReplica replica = e.getKey();
KafkaFuture future = e.getValue();
+
Review Comment:
nit: additional line
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINOR: enhance kafka-reassign-partitions command output [kafka]
AndrewJSchofield commented on code in PR #15610:
URL: https://github.com/apache/kafka/pull/15610#discussion_r1542090972
##
tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java:
##
@@ -806,12 +806,10 @@ private static void executeMoves(Admin adminClient,
do {
Set completed =
alterReplicaLogDirs(adminClient, pendingReplicas);
if (!completed.isEmpty()) {
-System.out.printf("Successfully started log directory move%s
for: %s%n",
-completed.size() == 1 ? "" : "s",
-completed.stream()
-
.sorted(ReassignPartitionsCommand::compareTopicPartitionReplicas)
-.map(Object::toString)
-.collect(Collectors.joining(",")));
+
completed.stream().sorted(ReassignPartitionsCommand::compareTopicPartitionReplicas).forEach(replica
-> {
+System.out.printf("Successfully started moving log
directory to %s for replica %s-%s with broker id: %s %n",
Review Comment:
This seems like a much nicer way to format the output. I suggest that `...
with broker %s%n` would be a bit neater.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
