gharris1727 commented on code in PR #16288:
URL: https://github.com/apache/kafka/pull/16288#discussion_r1635480089
##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/ConnectWorkerIntegrationTest.java:
##########
@@ -218,9 +218,6 @@ public void testRestartFailedTask() throws Exception {
props.put(TASKS_MAX_CONFIG, Objects.toString(numTasks));
props.put(CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX +
BOOTSTRAP_SERVERS_CONFIG, "nobrokerrunningatthisaddress");
Review Comment:
The first assertion in testAddAndRemoveWorker() can also be eliminated.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/SourceConnectorsIntegrationTest.java:
##########
@@ -160,8 +156,6 @@ public void testSwitchingToTopicCreationEnabled() throws
InterruptedException {
connect.assertions().assertTopicSettings(BAR_TOPIC,
DEFAULT_REPLICATION_FACTOR,
DEFAULT_PARTITIONS, "Topic " + BAR_TOPIC + " does not have the
expected settings");
- connect.assertions().assertAtLeastNumWorkersAreUp(NUM_WORKERS,
"Initial group of workers did not start in time.");
-
Map<String, String> barProps = defaultSourceConnectorProps(BAR_TOPIC);
Review Comment:
nit: there's an inaccurate assertAtLeastNumWorkersAreUp string later in this
test after the cluster is rolled.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/InternalTopicsIntegrationTest.java:
##########
@@ -169,7 +164,6 @@ public void
testFailToStartWhenInternalTopicsAreNotCompacted() throws Interrupte
// Start the brokers but not Connect
log.info("Starting {} Kafka brokers, but no Connect workers yet",
numBrokers);
connect.start();
Review Comment:
Oh interesting, this sets numWorkers=0, and therefore can call the blocking
start() method safely.
WDYT about changing
testFailToCreateInternalTopicsWithMoreReplicasThanBrokers to use the same
pattern? That would eliminate the need for the non-blocking start method, and
simplify the control flow.
The minority of call-sites are going to expect the workers to fail to start
up, so I think it's okay to use a workaround instead of giving them a
first-class method.
--
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]
