[GitHub] [kafka] rondagostino commented on a change in pull request #10286: KAFKA-12318: system tests need to fetch Topic IDs via Admin Client instead of via ZooKeeper

[GitBox <gi...@apache.org>]

rondagostino commented on a change in pull request #10286:
URL: https://github.com/apache/kafka/pull/10286#discussion_r590754821



##########
File path: tests/kafkatest/services/kafka/kafka.py
##########
@@ -1120,7 +1121,7 @@ def run_cli_tool(self, node, cmd):
 
     def parse_describe_topic(self, topic_description):
         """Parse output of kafka-topics.sh --describe (or describe_topic() method above), which is a string of form
-        PartitionCount:2\tReplicationFactor:2\tConfigs:
+        Topic:test_topic\tTopicId:<topic_id>\tPartitionCount:2\tReplicationFactor:2\tConfigs:

Review comment:
       Add the spaces?  (e.g. `Topic: test_topic\tTopicId: <topic_id>\tPartitionCount: 2\tReplicationFactor: 2\tConfigs:`
   
   Also, in next line, `s/tPartition/ Partition/` I think.

##########
File path: tests/kafkatest/version.py
##########
@@ -101,7 +101,7 @@ def kafka_configs_command_uses_bootstrap_server_scram(self):
         # User SCRAM Credentials (KIP-554)
         return self >= V_2_7_0
 
-    def supports_topic_ids(self):
+    def zk_supports_topic_ids(self):

Review comment:
       Maybe `supports_topic_ids_when_using_zk` is a better name since it isn't ZooKeeper that is supporting topic IDs?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org