You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jun Rao <ju...@gmail.com> on 2014/02/02 20:25:00 UTC
Re: Review Request 17479: Patch for KAFKA-1232
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/
-----------------------------------------------------------
(Updated Feb. 2, 2014, 7:24 p.m.)
Review request for kafka.
Bugs: KAFKA-1232
https://issues.apache.org/jira/browse/KAFKA-1232
Repository: kafka
Description (updated)
-------
1. Consolidated all arg checks in TopicCommandOptions.checkArgs() and added additional checks for invalid args. 2. Fixed deleteConfig to only expect deleted config names to be specified.
Address review comments.
Only support the topics-with-overrides option in describe topic to keep list topic simple.
1. Made the topic arg optional in both list topic and describe topic. If not specified, all topics will be used. 2. Made The topics-with-overrides option available in describe topic and remove the extra print out in list topic. 3. Added a check on mutually exclusive options.
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 842c11047cca0531fbc572fdb25523244ba2b626
core/src/main/scala/kafka/utils/CommandLineUtils.scala 5f563ca93ed5c6e4231a42331619ca41375844ca
Diff: https://reviews.apache.org/r/17479/diff/
Testing
-------
Thanks,
Jun Rao
Re: Review Request 17479: Patch for KAFKA-1232
Posted by Jun Rao <ju...@gmail.com>.
> On Feb. 3, 2014, 7:57 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/admin/TopicCommand.scala, line 275
> > <https://reviews.apache.org/r/17479/diff/3-4/?file=456676#file456676line275>
> >
> > shouldn't the first check be that exactly one of alter, create, delete, describe or list is used? That will simplify the rest of the checks
That check was already done at the beginning in main().
- Jun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/#review33484
-----------------------------------------------------------
On Feb. 2, 2014, 7:24 p.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2014, 7:24 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 1. Consolidated all arg checks in TopicCommandOptions.checkArgs() and added additional checks for invalid args. 2. Fixed deleteConfig to only expect deleted config names to be specified.
>
>
> Address review comments.
>
>
> Only support the topics-with-overrides option in describe topic to keep list topic simple.
>
>
> 1. Made the topic arg optional in both list topic and describe topic. If not specified, all topics will be used. 2. Made The topics-with-overrides option available in describe topic and remove the extra print out in list topic. 3. Added a check on mutually exclusive options.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 842c11047cca0531fbc572fdb25523244ba2b626
> core/src/main/scala/kafka/utils/CommandLineUtils.scala 5f563ca93ed5c6e4231a42331619ca41375844ca
>
> Diff: https://reviews.apache.org/r/17479/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jun Rao
>
>
Re: Review Request 17479: Patch for KAFKA-1232
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/#review33484
-----------------------------------------------------------
core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/17479/#comment62936>
shouldn't the first check be that exactly one of alter, create, delete, describe or list is used? That will simplify the rest of the checks
- Neha Narkhede
On Feb. 2, 2014, 7:24 p.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2014, 7:24 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 1. Consolidated all arg checks in TopicCommandOptions.checkArgs() and added additional checks for invalid args. 2. Fixed deleteConfig to only expect deleted config names to be specified.
>
>
> Address review comments.
>
>
> Only support the topics-with-overrides option in describe topic to keep list topic simple.
>
>
> 1. Made the topic arg optional in both list topic and describe topic. If not specified, all topics will be used. 2. Made The topics-with-overrides option available in describe topic and remove the extra print out in list topic. 3. Added a check on mutually exclusive options.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 842c11047cca0531fbc572fdb25523244ba2b626
> core/src/main/scala/kafka/utils/CommandLineUtils.scala 5f563ca93ed5c6e4231a42331619ca41375844ca
>
> Diff: https://reviews.apache.org/r/17479/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jun Rao
>
>
Re: Review Request 17479: Patch for KAFKA-1232
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/#review33404
-----------------------------------------------------------
Ship it!
Ship It!
- Guozhang Wang
On Feb. 2, 2014, 7:24 p.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2014, 7:24 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 1. Consolidated all arg checks in TopicCommandOptions.checkArgs() and added additional checks for invalid args. 2. Fixed deleteConfig to only expect deleted config names to be specified.
>
>
> Address review comments.
>
>
> Only support the topics-with-overrides option in describe topic to keep list topic simple.
>
>
> 1. Made the topic arg optional in both list topic and describe topic. If not specified, all topics will be used. 2. Made The topics-with-overrides option available in describe topic and remove the extra print out in list topic. 3. Added a check on mutually exclusive options.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 842c11047cca0531fbc572fdb25523244ba2b626
> core/src/main/scala/kafka/utils/CommandLineUtils.scala 5f563ca93ed5c6e4231a42331619ca41375844ca
>
> Diff: https://reviews.apache.org/r/17479/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jun Rao
>
>