AndrewJSchofield commented on code in PR #18829:
URL: https://github.com/apache/kafka/pull/18829#discussion_r1949060513


##########
tests/kafkatest/services/kafka/kafka.py:
##########
@@ -292,10 +292,18 @@ def __init__(self, context, num_nodes, zk, 
security_protocol=SecurityConfig.PLAI
                 use_new_coordinator = context.injected_args.get(arg_name)
             if use_new_coordinator is None:
                 use_new_coordinator = context.globals.get(arg_name)
+
+        # Set use_share_groups based on injected arguments.

Review Comment:
   Why is the handling of `use_share_groups` different than 
`use_transactions_v2`? It seems to me that this is different from both 
`use_new_coordinator` and `use_transactions_v2`, and I cannot see why it 
shouldn't just follow the simpler `use_transactions_v2` pattern.



##########
tests/kafkatest/services/kafka/kafka.py:
##########
@@ -778,6 +786,13 @@ def prop_file(self, node):
         for prop in self.per_node_server_prop_overrides.get(self.idx(node), 
[]):
             override_configs[prop[0]] = prop[1]
 
+        if self.use_share_groups:
+            override_configs[config_property.SHARE_GROUP_ENABLE] = 'true'
+            override_configs[config_property.UNSTABLE_API_VERSIONS_ENABLE] = 
'true'
+            
override_configs[config_property.GROUP_COORDINATOR_REBALANCE_PROTOCOLS] = 
'classic,consumer,share'
+            
override_configs[config_property.SHARE_COORDINATOR_STATE_TOPIC_REPLICATION_FACTOR]
 = str(self.num_nodes)

Review Comment:
   I'd set the maximum to 3 here. Potentially someone could run a test with 
lots of nodes, and there's no need for a massive replication factor to match.



-- 
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]

Reply via email to