[GitHub] [flink] zentol commented on a change in pull request #10724: [FLINK-13595][connector/kafka][test] Close the KafkaAdminClient with ...

[GitBox <gi...@apache.org>]

zentol commented on a change in pull request #10724: [FLINK-13595][connector/kafka][test] Close the KafkaAdminClient with ...
URL: https://github.com/apache/flink/pull/10724#discussion_r362825250
 
 

 ##########
 File path: flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaTestEnvironmentImpl.java
 ##########
 @@ -148,11 +150,19 @@ public void prepare(Config config) throws Exception {
 	@Override
 	public void deleteTestTopic(String topic) {
 		LOG.info("Deleting topic {}", topic);
-		try (AdminClient adminClient = AdminClient.create(getStandardProperties())) {
+		Properties props = getSecureProperties();
+		props.putAll(getStandardProperties());
+		String clientId = Long.toString(new Random().nextLong());
+		props.put("client.id", clientId);
+		AdminClient adminClient = AdminClient.create(props);
+		try {
 			tryDelete(adminClient, topic);
 		} catch (Exception e) {
 			e.printStackTrace();
 			fail(String.format("Delete test topic : %s failed, %s", topic, e.getMessage()));
+		} finally {
+			adminClient.close(Duration.ofMillis(30000L));
 
 Review comment:
   This is way too long. If our stance is that the shutdown is not relevant for correctness then let's set this to a low value (5 seconds max).
   
   Also add a comment that we don't use a try-with-resources so that we can introduce a timeout.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services