mjsax commented on code in PR #18160:
URL: https://github.com/apache/kafka/pull/18160#discussion_r1882852819
##########
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)
SG. I actually believe/hope, if we fix this inside `StreamsResetter` we
might not need to set the protocol here and can revert these changes in
`AbstractResetIntegrationTest` -- the fact that we need this changes here,
seems to surface the actual bug inside `StreamsResettter` and if we fix this
bug, we won't need it in the test setup any longer.
> wondering if we should "guard it from being overwritten"
The guard should be to disallow setting it to "consumer"... If one set's
"classic" expliclity, we don't need to do anything.
> 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.
In the end, this should not be necessary to begin with.
> 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?
I don't think we need to raise an error. If one sets it to "classic" we can
just accept it (ie, ignore it). And if one set's it to "consumer" we should
overwrite to "classic" and log a warning.
--
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]
