ivandasch commented on a change in pull request #9196: URL: https://github.com/apache/kafka/pull/9196#discussion_r475195007
########## File path: tests/kafkatest/tests/connect/connect_distributed_test.py ########## @@ -440,11 +441,11 @@ def test_bounce(self, clean, connect_protocol): sink_seqnos = [msg['seqno'] for msg in sink_messages if msg['task'] == task] # Every seqno up to the largest one we ever saw should appear. Each seqno should only appear once because # clean bouncing should commit on rebalance. - sink_seqno_max = max(sink_seqnos) + sink_seqno_max = max(sink_seqnos) if len(sink_seqnos) else 0 self.logger.debug("Max sink seqno: %d", sink_seqno_max) sink_seqno_counts = Counter(sink_seqnos) missing_sink_seqnos = sorted(set(range(sink_seqno_max)).difference(set(sink_seqnos))) - duplicate_sink_seqnos = sorted([seqno for seqno,count in sink_seqno_counts.iteritems() if count > 1]) + duplicate_sink_seqnos = sorted([seqno for seqno,count in iter(sink_seqno_counts.items()) if count > 1]) Review comment: Rewrite please it like this: `sorted(seqno for seqno,count in iter(sink_seqno_counts.items()) if count > 1)` ########## File path: tests/kafkatest/services/performance/producer_performance.py ########## @@ -78,7 +78,7 @@ def start_cmd(self, node): 'bootstrap_servers': self.kafka.bootstrap_servers(self.security_config.security_protocol), 'client_id': self.client_id, 'kafka_run_class': self.path.script("kafka-run-class.sh", node), - 'metrics_props': ' '.join(["%s=%s" % (k, v) for k, v in self.http_metrics_client_configs.iteritems()]) + 'metrics_props': ' '.join(["%s=%s" % (k, v) for k, v in self.http_metrics_client_configs.items()]) Review comment: You don't have to create list in order to use join, just ` ' '.join("%s=%s" % (k, v) for k, v in self.http_metrics_client_configs.items())` ########## File path: tests/kafkatest/services/monitor/http.py ########## @@ -154,7 +154,7 @@ def do_POST(self): name = raw_metric['name'] group = raw_metric['group'] # Convert to tuple of pairs because dicts & lists are unhashable - tags = tuple([(k, v) for k, v in raw_metric['tags'].iteritems()]), + tags = tuple([(k, v) for k, v in raw_metric['tags'].items()]), Review comment: You don't have to create list here, use generator as ctor args : `tags = tuple(raw_metric['tags'].items())` ########## File path: tests/kafkatest/version.py ########## @@ -49,6 +49,34 @@ def __str__(self): else: return LooseVersion.__str__(self) + def __eq__(self, other): + return self._cmp(other) == 0 + + def __lt__(self, other): + return self._cmp(other) < 0 + + def __le__(self, other): + return self._cmp(other) <= 0 + + def __gt__(self, other): + return self._cmp(other) > 0 + + def __ge__(self, other): + return self._cmp(other) >= 0 + + def _cmp(self, other): Review comment: Yes, but I don't understand why not call super methods properly, this approach is outdated even for python 2.7 ########## File path: tests/kafkatest/tests/connect/connect_distributed_test.py ########## @@ -420,11 +421,11 @@ def test_bounce(self, clean, connect_protocol): src_seqnos = [msg['seqno'] for msg in src_messages if msg['task'] == task] # Every seqno up to the largest one we ever saw should appear. Each seqno should only appear once because clean # bouncing should commit on rebalance. - src_seqno_max = max(src_seqnos) + src_seqno_max = max(src_seqnos) if len(src_seqnos) else 0 self.logger.debug("Max source seqno: %d", src_seqno_max) src_seqno_counts = Counter(src_seqnos) missing_src_seqnos = sorted(set(range(src_seqno_max)).difference(set(src_seqnos))) - duplicate_src_seqnos = sorted([seqno for seqno,count in src_seqno_counts.iteritems() if count > 1]) + duplicate_src_seqnos = sorted([seqno for seqno,count in src_seqno_counts.items() if count > 1]) Review comment: Rewrite please it like this: sorted(seqno for seqno,count in iter(sink_seqno_counts.items()) if count > 1) ---------------------------------------------------------------- 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: us...@infra.apache.org