chia7712 commented on code in PR #17673:
URL: https://github.com/apache/kafka/pull/17673#discussion_r1834649947
##########
tests/kafkatest/sanity_checks/test_verifiable_producer.py:
##########
@@ -91,10 +87,7 @@ def test_simple_run(self, producer_version, acks=None,
enable_idempotence=False,
# the time it takes the producer to produce its messages.
Review Comment:
those comments are out-of-date after removing the 0.8.2, right?
##########
tests/kafkatest/sanity_checks/test_performance_services.py:
##########
@@ -37,16 +37,9 @@ def setUp(self):
if self.zk:
self.zk.start()
- @cluster(num_nodes=5)
- # We are keeping 0.8.2 here so that we don't inadvertently break support
for it. Since this is just a sanity check,
- # the overhead should be manageable.
- @parametrize(version=str(LATEST_0_8_2), new_consumer=False)
- @parametrize(version=str(LATEST_0_9), new_consumer=False)
- @parametrize(version=str(LATEST_0_9))
- @parametrize(version=str(LATEST_1_1), new_consumer=False)
Review Comment:
we should keep one test suit for `LATEST_2_1` with old consumer
##########
tests/kafkatest/services/console_consumer.py:
##########
@@ -209,16 +193,14 @@ def start_cmd(self, node):
cmd += " --property print.partition=true"
# LoggingMessageFormatter was introduced after 0.9
- if node.version > LATEST_0_9:
- if node.version > LATEST_3_7:
- cmd += " --formatter
org.apache.kafka.tools.consumer.LoggingMessageFormatter"
- else:
- cmd += " --formatter kafka.tools.LoggingMessageFormatter"
+ if node.version > LATEST_3_7:
+ cmd += " --formatter
org.apache.kafka.tools.consumer.LoggingMessageFormatter"
+ else:
+ cmd += " --formatter kafka.tools.LoggingMessageFormatter"
if self.enable_systest_events:
# enable systest events is only available in 0.10.0 and later
Review Comment:
those comments are unused now, right?
##########
tests/kafkatest/version.py:
##########
@@ -72,10 +72,10 @@ def consumer_supports_bootstrap_server(self):
:return: true if the version of Kafka supports a new consumer with
--bootstrap-server
"""
- return self >= V_0_9_0_0
+ return True
Review Comment:
we don't need `consumer_supports_bootstrap_server`, right?
##########
tests/kafkatest/version.py:
##########
@@ -72,10 +72,10 @@ def consumer_supports_bootstrap_server(self):
:return: true if the version of Kafka supports a new consumer with
--bootstrap-server
"""
- return self >= V_0_9_0_0
+ return True
def supports_named_listeners(self):
Review Comment:
ditto
##########
tests/kafkatest/services/performance/consumer_performance.py:
##########
@@ -77,9 +77,6 @@ def __init__(self, context, num_nodes, kafka, topic,
messages, version=DEV_BRANC
assert version.consumer_supports_bootstrap_server() or (not
new_consumer), \
"new_consumer is only supported if version >= 0.9.0.0, version %s"
% str(version)
- assert version < V_2_0_0 or new_consumer, \
Review Comment:
`new_consumer` is always `true`, right?
--
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]