KAFKA-3673: Connect tests don't handle concurrent config changes #1340
Conversation
|
@ewencp @hachikuji Ready for review. Thanks! |
| @@ -65,7 +66,9 @@ def test_rest_api(self): | |||
| sink_connector_props = self.render("connect-file-sink.properties") | |||
| for connector_props in [source_connector_props, sink_connector_props]: | |||
| connector_config = self._config_dict_from_props(connector_props) | |||
| self.cc.create_connector(connector_config) | |||
| wait_until(lambda: self.not_throw_rest_error(lambda: self.cc.create_connector(connector_config)), timeout_sec=120, err_msg="Create connectors throws exception.") | |||
There was a problem hiding this comment.
I think we might need to address the problem more generally. Technically you can get 409 errors on pretty much any call due to rebalances, right? Or did you choose to protect the specific API calls you did for a reason? Are there only certain cases that can trigger rebalances in this test given that we aren't bouncing any workers and therefore only protecting a subset of them is safe?
I'm wondering if the approach to retrying should be flipped around, where every API call has something like a retries parameter. That would allow for easily indicating you expect immediate success (retries=0) or that you want to account for possible rebalances (retries=some larger number).
|
@ewencp Addressed review comments. PTAL. |
|
LGTM, w/ relevant system test passing here: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/439/console |
Author: Liquan Pei <liquanpei@gmail.com> Reviewers: Ewen Cheslack-Postava <ewen@confluent.io> Closes apache#1340 from Ishiihara/connect-test-failure
No description provided.