showuon commented on code in PR #15659:
URL: https://github.com/apache/kafka/pull/15659#discussion_r1554446742
##########
tools/src/test/java/org/apache/kafka/tools/reassign/ReassignPartitionsUnitTest.java:
##########
@@ -300,6 +300,12 @@ public void testGetReplicaAssignments() throws Exception {
assertEquals(assignments,
getReplicaAssignmentForPartitions(adminClient, new
HashSet<>(asList(new TopicPartition("foo", 0), new TopicPartition("bar", 0)))));
+
+ assignments.clear();
+
+ UnknownTopicOrPartitionException exception =
assertThrows(UnknownTopicOrPartitionException.class,
+ () -> getReplicaAssignmentForPartitions(adminClient, new
HashSet<>(asList(new TopicPartition("foo", 0), new TopicPartition("foo",
10)))));
Review Comment:
In `ReassignPartitionsCommand#describeTopics`, we wrap
`UnknownTopicOrPartitionException` with `ExecutionException`. Maybe we should
make it consistent by this pattern?
##########
tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java:
##########
@@ -654,6 +654,13 @@ static Map<TopicPartition, List<Integer>>
getReplicaAssignmentForPartitions(Admi
res.put(tp,
info.replicas().stream().map(Node::id).collect(Collectors.toList()));
})
);
+
+ if (!res.keySet().equals(partitions)) {
+ Set<TopicPartition> missingPartitions = new HashSet<>(partitions);
+ missingPartitions.removeAll(res.keySet());
+ throw new UnknownTopicOrPartitionException("Unable to find
partition: " +
+
missingPartitions.stream().map(TopicPartition::toString).collect(Collectors.joining(",
")));
Review Comment:
In ReassignPartitionsCommand#describeTopics, we wrap
UnknownTopicOrPartitionException with ExecutionException. Maybe we should make
it consistent by this pattern?
--
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
