KAFKA-2603: Add timeout arg to ConsoleConsumer #274
Conversation
There was a problem hiding this comment.
It should be ok to throw an Exception if a negative timeout is specified
There was a problem hiding this comment.
@auradkar The timeout parameter is consistent with "consumer.timeout.ms" config option in the old consumer. With the old consumer, timeout less than zero indicates that there is no timeout. The same behaviour is used here for consistency.
48be188 to
79090d7
Compare
|
Just realize now console_consumer.properties would become a dummy file again :) My take is that the value of having a template file is to specify some universal config values that will be used across all test runs with probably different combinations of other config values; or universally use some pre-defined values from the framework setup. For example tests/kafkatest/services/templates/console_consumer.properties If we do not have any value definitions in the template file (i.e. a file is empty / dummy), then appending different values to this file and use it for various test runs is no difference with creating a new file each run with different values on-the-fly. Hence I feel the empty file itself brings us no extra benefit for customizing test runs, and better be removed and creating new files for different runs instead. For now since we do not have customizable test runs using console consumer yet we can simply do the same as #271. |
79090d7 to
529170e
Compare
|
@guozhangwang Have removed the empty console_consumer.properties file in the latest commit. |
…d new consumer, use arg in ducktape service
c8e1ad1 to
5764420
Compare
|
console_consumer.properties is now used by quota tests and is no longer empty. So I have reverted the change to remove the file and rebased. |
|
LGTM, thanks @rajinisivaram for being so patient in the review process. |
|
@guozhangwang Thank you for reviewing and committing this. |
Added --timeout-ms argument to ConsoleConsumer that works with both old and new consumer. Also modified ducktape ConsoleConsumer service to use this arg instead of consumer.timeout.ms config that works only with the old consumer.