KAFKA-5599: ConsoleConsumer : --new-consumer option as deprecated #3537
Conversation
Printing a warning on using the new consumer but adding the --new-consumer option
|
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): |
| @@ -269,7 +269,7 @@ object ConsoleConsumer extends Logging { | |||
| .withRequiredArg | |||
| .describedAs("metrics directory") | |||
| .ofType(classOf[java.lang.String]) | |||
| val newConsumerOpt = parser.accepts("new-consumer", "Use the new consumer implementation. This is the default.") | |||
| val newConsumerOpt = parser.accepts("new-consumer", "DEPRECATED Use the new consumer implementation. This is the default.") | |||
There was a problem hiding this comment.
Maybe at the end, "This is the default, so this option is deprecated and will be removed in a future release." Otherwise it may seem like using the new consumer is deprecated.
| CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt) | ||
|
|
||
| if (options.has(newConsumerOpt)) { | ||
| Console.err.println("Using the --new-consumer option is deprecated and will be removed " + | ||
| "in a future major release. For using the new consumer just use only the --bootstrap-server option") |
There was a problem hiding this comment.
Maybe:
"The --new-consumer option is deprecated and will be removed in a future major release. The new consumer is used by default if the --bootstrap-server option is provided."
|
@ijuma updted after your comments. As native english speaker I think you are absolutely right ;) For the other tools which use the --new-consumer option I'm going to open a new JIRA and the providing a different PR. |
|
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): |
|
Retest this please |
|
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): |
|
@ijuma any news about having this merged after my changes ? |
|
This PR should be ready to merge after the @ijuma approval. What do you think @guozhangwang ? I have a quite similar PR #3555 for all other tools. It could be good to have both merged for the 1.0.0 release so we can then move to remove this option in the next release cycle. |
|
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 @ppatierno . I'll leave to @ijuma to merge then since he reviewed the PR. |
| @@ -269,7 +269,7 @@ object ConsoleConsumer extends Logging { | |||
| .withRequiredArg | |||
| .describedAs("metrics directory") | |||
| .ofType(classOf[java.lang.String]) | |||
| val newConsumerOpt = parser.accepts("new-consumer", "Use the new consumer implementation. This is the default.") | |||
| val newConsumerOpt = parser.accepts("new-consumer", "This is the default, so this option is deprecated and will be removed in a future release.") | |||
There was a problem hiding this comment.
We should still have the description of the parameter. I'll add it back before merging.
Marking --new-consumer as deprecated in the help of ConsoleConsumer
Printing a warning on using the new consumer but adding the --new-consumer option