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/01/29 03:50:20 UTC
Review Request 17479: make TopicCommand more consistent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1232
https://issues.apache.org/jira/browse/KAFKA-1232
Repository: kafka
Description
-------
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/#review33230
-----------------------------------------------------------
Ship it!
Ship It!
- Neha Narkhede
On Jan. 30, 2014, 5:51 p.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 5:51 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 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/#review33338
-----------------------------------------------------------
Ship it!
Ship It!
- Guozhang Wang
On Jan. 30, 2014, 5:51 p.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 5:51 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 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 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
>
>
Re: Review Request 17479: Patch for KAFKA-1232
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/
-----------------------------------------------------------
(Updated Jan. 30, 2014, 5:51 p.m.)
Review request for kafka.
Bugs: KAFKA-1232
https://issues.apache.org/jira/browse/KAFKA-1232
Repository: kafka
Description (updated)
-------
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 Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/#review33128
-----------------------------------------------------------
core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/17479/#comment62411>
Since these three options are exclusive, shall we make these three as separate code paths to be more clean?
- Guozhang Wang
On Jan. 29, 2014, 5:21 a.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2014, 5:21 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 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/#review33135
-----------------------------------------------------------
core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/17479/#comment62426>
Do you need both getSpecifiedTopics or getSpecifiedOrAllTopics? Can getSpecifiedTopics take in a boolean to indicate whether all topics can be returned or not, if the topic option is not specified?
core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/17479/#comment62427>
This was done to provide a means to compare topic metadata across 2 mirrors. To faciliate that, we need to dump topic metadata that is independent of broker metadata. For example, topic, # of partitions and replication factor, but not where those replicas are, ISR etc since that precludes doing a unix simple diff between the topic output of 2 mirror clusters.
- Neha Narkhede
On Jan. 29, 2014, 5:21 a.m., Jun Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17479/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2014, 5:21 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1232
> https://issues.apache.org/jira/browse/KAFKA-1232
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 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 Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17479/
-----------------------------------------------------------
(Updated Jan. 29, 2014, 5:21 a.m.)
Review request for kafka.
Summary (updated)
-----------------
Patch for KAFKA-1232
Bugs: KAFKA-1232
https://issues.apache.org/jira/browse/KAFKA-1232
Repository: kafka
Description (updated)
-------
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