cmccabe commented on code in PR #14678:
URL: https://github.com/apache/kafka/pull/14678#discussion_r1380540025
##########
core/src/test/java/kafka/testkit/TestKitNodes.java:
##########
@@ -119,7 +101,35 @@ public TestKitNodes build() {
if (bootstrapMetadataVersion == null) {
bootstrapMetadataVersion = MetadataVersion.latest();
}
- return new TestKitNodes(clusterId, bootstrapMetadataVersion,
controllerNodes, brokerNodes);
+ String baseDirectory = TestUtils.tempDirectory("kafka_" +
clusterId).getAbsolutePath();
+ try {
+ NavigableMap<Integer, ControllerNode> controllerNodes = new
TreeMap<>();
+ for (ControllerNode.Builder controllerNodeBuilder :
controllerNodeBuilders.values()) {
+ ControllerNode controllerNode =
controllerNodeBuilder.build(baseDirectory);
+ if (controllerNodes.put(controllerNode.id(),
controllerNode) != null) {
+ throw new RuntimeException("More than one controller
claimed ID " + controllerNode.id());
+ }
+ }
+ NavigableMap<Integer, BrokerNode> brokerNodes = new
TreeMap<>();
+ for (BrokerNode.Builder brokerNodeBuilder :
brokerNodeBuilders.values()) {
+ BrokerNode brokerNode =
brokerNodeBuilder.build(baseDirectory);
+ if (brokerNodes.put(brokerNode.id(), brokerNode) != null) {
+ throw new RuntimeException("More than one broker
claimed ID " + brokerNode.id());
+ }
+ }
+ return new TestKitNodes(baseDirectory,
+ clusterId,
+ bootstrapMetadataVersion,
+ controllerNodes,
+ brokerNodes);
+ } catch (Exception e) {
Review Comment:
Yeah, that's fair. We can only have `RuntimException` here. Still, I can see
no logical reason why we wouldn't do the deletion if the code was restructured
to throw checked exceptions here. So it seems clearer this way.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
