gharris1727 commented on code in PR #14313: URL: https://github.com/apache/kafka/pull/14313#discussion_r1312082806
########## connect/runtime/src/test/java/org/apache/kafka/connect/util/TopicAdminTest.java: ########## @@ -532,19 +532,31 @@ public void retryEndOffsetsShouldWrapNonRetriableExceptionsWithConnectException( Set<TopicPartition> tps = Collections.singleton(tp1); Long offset = 1000L; Cluster cluster = createCluster(1, "myTopic", 1); + String bootstrapServers = "localhost:8121"; - try (final AdminClientUnitTestEnv env = new AdminClientUnitTestEnv(new MockTime(10), cluster)) { + try (final AdminClientUnitTestEnv env = new AdminClientUnitTestEnv(new MockTime(), cluster)) { Map<TopicPartition, Long> offsetMap = new HashMap<>(); offsetMap.put(tp1, offset); env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); - env.kafkaClient().prepareResponse(prepareMetadataResponse(cluster, Errors.UNKNOWN_TOPIC_OR_PARTITION, Errors.NONE)); - Map<String, Object> adminConfig = new HashMap<>(); - adminConfig.put(AdminClientConfig.RETRY_BACKOFF_MS_CONFIG, "0"); - TopicAdmin admin = new TopicAdmin(adminConfig, env.adminClient()); - assertThrows(ConnectException.class, () -> { - admin.retryEndOffsets(tps, Duration.ofMillis(100), 1); - }); + // This error should be treated as non-retriable and cause TopicAdmin::retryEndOffsets to fail + env.kafkaClient().prepareResponse(prepareMetadataResponse(cluster, Errors.TOPIC_AUTHORIZATION_FAILED, Errors.NONE)); + // But, in case there's a bug in our logic, prepare a valid response afterward so that TopicAdmin::retryEndOffsets + // will return successfully if we retry (which should in turn cause this test to fail) + env.kafkaClient().prepareResponse(prepareMetadataResponse(cluster, Errors.NONE)); + env.kafkaClient().prepareResponse(listOffsetsResult(tp1, offset)); + + TopicAdmin admin = new TopicAdmin(bootstrapServers, env.adminClient()); Review Comment: This wasn't introduced in this PR but it did draw my attention to it: This Object argument for TopicAdmin feels very strange to me, and I see that it is only used in log messages, so it is left as `null` in most tests. Perhaps we can follow the `null` convention in this test, and in a follow-up try to eliminate the Object argument. -- 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