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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
