[GitHub] [kafka] badaiaqrandista commented on pull request #9099: KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter
badaiaqrandista commented on pull request #9099: URL: https://github.com/apache/kafka/pull/9099#issuecomment-703939623 @bbejeck Do I need to do anything on this? 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
[GitHub] [kafka] badaiaqrandista commented on pull request #9099: KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter
badaiaqrandista commented on pull request #9099: URL: https://github.com/apache/kafka/pull/9099#issuecomment-679398619 @bbejeck It failed because it print the partition number as a single integer after the value. I moved the partition to be before the key (if printed) and value, and also added prefix "Partition:" to differentiate it from "Offset:". It's only printed if "print.partition=true". I will keep the tests as is then. ConsoleConsumerTest is a unit test for the class while DefaultMessageFormatterTest is more of an integration test. 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
[GitHub] [kafka] badaiaqrandista commented on pull request #9099: KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter
badaiaqrandista commented on pull request #9099: URL: https://github.com/apache/kafka/pull/9099#issuecomment-678834247 @bbejeck One question: As part of my PR, I add a new test for DefaultMessageFormatter class: core/src/test/scala/kafka/tools/DefaultMessageFormatterTest.scala But I found that there is another unit test that include a test for DefaultMessageFormatter class: https://github.com/apache/kafka/blob/trunk/core/src/test/scala/unit/kafka/tools/ConsoleConsumerTest.scala#L501 This was the cause of the system test failure yesterday. I changed the latter test to work with the updated DefaultMessageFormatter. But after thinking about it further, I should just delete the test for DefaultMessageFormatter in ConsoleConsumerTest.scala. What do you think? 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
[GitHub] [kafka] badaiaqrandista commented on pull request #9099: KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter
badaiaqrandista commented on pull request #9099: URL: https://github.com/apache/kafka/pull/9099#issuecomment-678641314 @bbejeck Thank you for including the changes in the test. I've fixed the errors found by the test. Please test again. 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
[GitHub] [kafka] badaiaqrandista commented on pull request #9099: KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter
badaiaqrandista commented on pull request #9099: URL: https://github.com/apache/kafka/pull/9099#issuecomment-673156754 @dajac Can you please re-review the changes I've made? 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
