KAFKA-5225: StreamsResetter tool to allow custom consumer properties #3062
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Thanks for the patch. I think, we should try to align this with other CLI tools. For example. I think, we also need a KIP for this JIRA. \cc @gwenshap @ewencp @guozhangwang |
|
@mjsax KIP is accurate, though this is one of those things that we should probably get a KIP for a standard set of config options across all tools so additions like this can just fall under the umbrella of that KIP... |
|
@mjsax Update the code to add consumer-property option and also changed the file option to consumer.config |
|
@ewencp @mjsax Proposed KIP 157 for this. Please let me know how to include this KIP 157 under main KIP for config options for kafka tools. As KIP 156 is also to add an option to reset tool. As if we include all KIP's related to tools under one main KIP, it will be easy to refer in future and also it will help further developers to use the similar naming convention for options in kafka tools. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
@mjsax @guozhangwang Could you please review the changes. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
| parsedConsumerProperties.put(property[0], property[1]); | ||
| } else { | ||
| System.err.println("Invalid command line properties: " + consumerProperties.toString()); | ||
| System.exit(1); |
There was a problem hiding this comment.
We should just throw an "InvalidArgumentException" here.
| @@ -163,6 +193,15 @@ private void parseArguments(final String[] args) throws IOException { | |||
| .ofType(String.class) | |||
| .withValuesSeparatedBy(',') | |||
| .describedAs("list"); | |||
| consumerPropertyOption = optionParser.accepts("consumer-property", "A mechanism to pass user-defined properties in the form key=value to the consumer") | |||
There was a problem hiding this comment.
We are using AdminClient, ZkUtils, and a KafkaConsumer -- do we need to allow properties for all three?
| @@ -163,6 +193,15 @@ private void parseArguments(final String[] args) throws IOException { | |||
| .ofType(String.class) | |||
| .withValuesSeparatedBy(',') | |||
| .describedAs("list"); | |||
| consumerPropertyOption = optionParser.accepts("consumer-property", "A mechanism to pass user-defined properties in the form key=value to the consumer") | |||
| .withRequiredArg() | |||
| .ofType(String.class) | |||
There was a problem hiding this comment.
Should this be a list-of-string instead of a string?
|
Created a new PR #3117 |
@mjsax @guozhangwang Could you please review the changes.