KAFKA-19202: Enable KIP-1071 in streams_smoke_test.py #19560
Conversation
github-actions
bot
added
tests
|
PTAL @aliehsaeedii |
| override_configs[config_property.GROUP_COORDINATOR_REBALANCE_PROTOCOLS] = 'classic,consumer,share' | ||
| elif self.use_streams_groups is True: | ||
| override_configs[config_property.UNSTABLE_API_VERSIONS_ENABLE] = str(True) | ||
| override_configs[config_property.GROUP_COORDINATOR_REBALANCE_PROTOCOLS] = 'classic,consumer,streams' |
There was a problem hiding this comment.
Would it be simpler to start with enabledProtocols = 'classic,consumer' and do
if self.use_streams_groups is True:
enabledProtocols += ',streams`
and end with
override_configs[config_property.GROUP_COORDINATOR_REBALANCE_PROTOCOLS] = enabledProtocols
Similar for share group? To avoid all this complicated nesting?
| self.num_brokers = num_brokers | ||
| self.topics = topics | ||
|
|
||
| self.zk = ZookeeperService(test_context, self.num_zk) if quorum.for_test(test_context) == quorum.zk else None |
There was a problem hiding this comment.
I'm just preserving the code that is already there (in KafkaTest). the quorum parameter is still everywhere in the streams tests. I can push a clean-up PR, for this PR, I just preserved what's tehre.
There was a problem hiding this comment.
Okay, removed it. I suppose quorum=zk is not used, so no need to support it here.
There was a problem hiding this comment.
Thanks. Might be a good idea to also go over other tests and clean this up, if it's still there?
I would only expect some upgrade/downgrade tests which use older broker versions to still use ZK parameter.
|
@mjsax This is ready for re-review |
Enables KIP-1071 (
group.protocol=streams) in the first streams systemtest
streams_smoke_test.py.All tests using KIP-1071 cannot use
KafkaTestanymore, since we needto customize the broker configuration. The corresponding functionality
is added to
BaseStreamsTest, which all streams tests will have toextend from now on.
There are some left-overs from ZK in the tests that I copied from
'KafkaTest'. They need to be cleaned up, but this should be done in a
separate PR.