C0urante commented on a change in pull request #8928:
URL: https://github.com/apache/kafka/pull/8928#discussion_r449797010
##########
File path:
connect/runtime/src/test/java/org/apache/kafka/connect/integration/BlockingConnectorTest.java
##########
@@ -76,6 +78,15 @@ public void setup() {
// start the clusters
connect.start();
+
+ // wait for the Connect REST API to become available. necessary
because of the reduced REST
+ // request timeout; otherwise, we may get an unexpected 500 with our
first real REST request
+ // if the worker is still getting on its feet.
+ waitForCondition(
Review comment:
I didn't change the assertions initially for two reasons:
1. This test is the only one that artificially reduces the REST request
timeout from the usual 90 seconds to just 5 seconds; it seemed fairly unlikely
that the distinction between "any REST request is ready to be handled by the
herder" and "all REST requests are ready to be handled by the herder" would
matter in any test that still uses the normal 90 second timeout. Herders not
starting in 5 seconds isn't too surprising; herders not starting in 90 seconds
is probably a sign of something going wrong.
2. The solution here, while valid for this test, is a little hacky. Issuing
a request for info of a non-existent connector and waiting for the response
status to transition from 500 to 404 works if you know that the connector
doesn't exist, but not necessarily if you aren't certain that that connector
shouldn't exist. We'd probably want to wait for the status to become either 404
(herder has completed startup and connector doesn't exist) or 200 (herder has
completed startup and connector does exist) and use some obscure name like
`"test-for-herder-startup"`, but even then, it's possible that there may be
unexpected side effects to these requests if they're used for all integration
tests instead of just ones where the conditions are more controlled.
LMK what you think; I'm willing to add this logic to the embedded cluster
assertions if there's a case to be made for how it'd benefit more than just
this test.
----------------------------------------------------------------
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:
us...@infra.apache.org
