chia7712 commented on code in PR #18036:
URL: https://github.com/apache/kafka/pull/18036#discussion_r1874533537
##########
tools/src/main/java/org/apache/kafka/tools/VerifiableConsumer.java:
##########
@@ -649,8 +648,12 @@ public static VerifiableConsumer
createFromArgs(ArgumentParser parser, String[]
if (groupRemoteAssignor != null)
consumerProps.put(ConsumerConfig.GROUP_REMOTE_ASSIGNOR_CONFIG,
groupRemoteAssignor);
} else {
- // This means we're using the old consumer group protocol.
+ // This means we're using the CLASSIC consumer group protocol.
consumerProps.put(ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG,
res.getString("assignmentStrategy"));
+ Integer sessionTimeout = res.getInt("sessionTimeout");
Review Comment:
If this tool does not offer the default value, it is fine to throw exception
if user does set the `--session-timeout`
##########
tests/kafkatest/services/verifiable_consumer.py:
##########
@@ -417,10 +417,13 @@ def start_cmd(self, node):
else:
cmd += " --bootstrap-server %s" %
self.kafka.bootstrap_servers(self.security_config.security_protocol)
- cmd += " --reset-policy %s --group-id %s --topic %s --session-timeout
%s" % \
- (self.reset_policy, self.group_id, self.topic,
- self.session_timeout_sec*1000)
-
+ cmd += " --reset-policy %s --group-id %s --topic %s" % \
+ (self.reset_policy, self.group_id, self.topic)
+
+ # session timeout is not supported when using CONSUMER group protocol
+ if self.session_timeout_sec > 0 and
self.is_consumer_group_protocol_enabled():
Review Comment:
this will swallow the incorrect configs in e2e. We can just set the session
timeout if e2e test case does set it.
--
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]
