lianetm commented on code in PR #18160:
URL: https://github.com/apache/kafka/pull/18160#discussion_r1882830570
##########
tools/src/test/java/org/apache/kafka/tools/AbstractResetIntegrationTest.java:
##########
@@ -406,6 +408,7 @@ protected boolean tryCleanGlobal(final boolean
withIntermediateTopics,
final Properties cleanUpConfig = new Properties();
cleanUpConfig.put(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG, 100);
cleanUpConfig.put(ConsumerConfig.SESSION_TIMEOUT_MS_CONFIG,
Integer.toString(CLEANUP_CONSUMER_TIMEOUT));
+ cleanUpConfig.put(ConsumerConfig.GROUP_PROTOCOL_CONFIG,
GroupProtocol.CLASSIC.name);
Review Comment:
I see. Definitely sounds sensible to add a validation for the protocol in
the `StreamsResetter` tool, and set it to classic there too (I can do it in a
follow-up PR, let's just agree on what exactly you guys think it's best for the
resetter tool experience)
I agree with your suggestion, only wondering if we should "guard it from
being overwritten"? In cases like this test example, that sets properties that
are strictly for the classic-protocol, it seems expected that one would
naturally set the protocol to classic, along with the HB and session. If we add
a guard, the user would get an error, which will seem noisy right? It's
definitely a config not needed but it's not wrong. Thoughts?
--
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]
