You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "JimmyWang6 (via GitHub)" <gi...@apache.org> on 2025/09/03 18:52:02 UTC
[PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
JimmyWang6 opened a new pull request, #20471:
URL: https://github.com/apache/kafka/pull/20471
#20390 Replace the -`-producer.config` for the verifiable producer and `--consumer.config` option by `--command-config` for the verifiable consumer. However, for e2e tests targeting older broker versions, the original configuration should still be used.
Fix the following tests: `consumer_protocol_migration_test.py`、`compatibility_test_new_broker_test.py` and `upgrade_test.py`.
My local test result before change:
<img width="905" height="447" alt="image" src="https://github.com/user-attachments/assets/d043ac61-6135-40d2-84d6-4364ef2b2be6" />
After:
<img width="917" height="393" alt="image" src="https://github.com/user-attachments/assets/495b3033-2b3d-432b-b8fb-6afbee50e203" />
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
Posted by "omkreddy (via GitHub)" <gi...@apache.org>.
omkreddy merged PR #20471:
URL: https://github.com/apache/kafka/pull/20471
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
Posted by "JimmyWang6 (via GitHub)" <gi...@apache.org>.
JimmyWang6 commented on code in PR #20471:
URL: https://github.com/apache/kafka/pull/20471#discussion_r2321686253
##########
tests/kafkatest/services/verifiable_consumer.py:
##########
@@ -424,7 +424,16 @@ def start_cmd(self, node):
if self.max_messages > 0:
cmd += " --max-messages %s" % str(self.max_messages)
- cmd += " --command-config %s" % VerifiableConsumer.CONFIG_FILE
+ version = get_version(node)
+ # According to KIP-1147, --consumer.config has been deprecated and will be removed in future versions.
Review Comment:
@omkreddy Thanks for your comments, sorry for my carelessness and I have addressed them.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
Posted by "JimmyWang6 (via GitHub)" <gi...@apache.org>.
JimmyWang6 commented on code in PR #20471:
URL: https://github.com/apache/kafka/pull/20471#discussion_r2320983054
##########
tests/kafkatest/services/verifiable_producer.py:
##########
@@ -249,7 +249,14 @@ def start_cmd(self, node, idx):
if self.repeating_keys is not None:
cmd += " --repeating-keys %s " % str(self.repeating_keys)
- cmd += " --command-config %s" % VerifiableProducer.CONFIG_FILE
+ # According to KIP-1147, --producer.config has been deprecated and will be removed in future versions.
+ # For backward compatibility, we select the configuration based on node version:
+ # - For versions 4.2.0 and above, use --command-config
+ # - For older versions, continue using --producer.config
+ if node.version >= V_4_2_0:
Review Comment:
Good idea. I have changed to use the new helper method.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
Posted by "omkreddy (via GitHub)" <gi...@apache.org>.
omkreddy commented on code in PR #20471:
URL: https://github.com/apache/kafka/pull/20471#discussion_r2321631002
##########
tests/kafkatest/services/verifiable_consumer.py:
##########
@@ -424,7 +424,16 @@ def start_cmd(self, node):
if self.max_messages > 0:
cmd += " --max-messages %s" % str(self.max_messages)
- cmd += " --command-config %s" % VerifiableConsumer.CONFIG_FILE
+ version = get_version(node)
+ # According to KIP-1147, --consumer.config has been deprecated and will be removed in future versions.
Review Comment:
Can we move these comments to version.supports_command_config() method?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20471:
URL: https://github.com/apache/kafka/pull/20471#discussion_r2319940997
##########
tests/kafkatest/services/verifiable_producer.py:
##########
@@ -249,7 +249,14 @@ def start_cmd(self, node, idx):
if self.repeating_keys is not None:
cmd += " --repeating-keys %s " % str(self.repeating_keys)
- cmd += " --command-config %s" % VerifiableProducer.CONFIG_FILE
+ # According to KIP-1147, --producer.config has been deprecated and will be removed in future versions.
+ # For backward compatibility, we select the configuration based on node version:
+ # - For versions 4.2.0 and above, use --command-config
+ # - For older versions, continue using --producer.config
+ if node.version >= V_4_2_0:
Review Comment:
Could you please consider adding a helper method to `KafkaVersion`?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix failed e2e compatibility_test_new_broker_test and upgrade_test.py [kafka]
Posted by "omkreddy (via GitHub)" <gi...@apache.org>.
omkreddy commented on code in PR #20471:
URL: https://github.com/apache/kafka/pull/20471#discussion_r2321631002
##########
tests/kafkatest/services/verifiable_consumer.py:
##########
@@ -424,7 +424,16 @@ def start_cmd(self, node):
if self.max_messages > 0:
cmd += " --max-messages %s" % str(self.max_messages)
- cmd += " --command-config %s" % VerifiableConsumer.CONFIG_FILE
+ version = get_version(node)
+ # According to KIP-1147, --consumer.config has been deprecated and will be removed in future versions.
Review Comment:
Can we move this comment to version.supports_command_config() method?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org