Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51884 --- Ship it! Thanks for the patch. +1 with the following minor comments

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/ --- (Updated Aug. 28, 2014, 11:20 p.m.) Review request for kafka. Bugs: KAFKA-687

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 28, 2014, 1:32 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, line > > 721 > > > > > > Could we use case to avoid _._1 ? > > > > Also, could you explai

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 28, 2014, 1:32 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/consumer/ConsumerConfig.scala, line 52 > > > > > > This probably should be named DefaultPartitionAssignmentStrategy. Ok - it seems a number of

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51694 --- core/src/main/scala/kafka/consumer/ConsumerConfig.scala

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 23, 2014, 9:57 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/consumer/PartitionAssignor.scala, line 159 > > > > > > It may be more helpful to log the partition results in the end than > > logging

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51432 --- clients/src/main/java/org/apache/kafka/common/requests/MetadataResp

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/ --- (Updated Aug. 25, 2014, 7:36 p.m.) Review request for kafka. Bugs: KAFKA-687

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 23, 2014, 10:03 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/consumer/PartitionAssignorTest.scala, lines > > 85-86 > > > > > > Can we use a more meaningful name here, e.g. consumerIndex? I'm

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 23, 2014, 9:57 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/consumer/PartitionAssignor.scala, line 159 > > > > > > It may be more helpful to log the partition results in the end than > > logging

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 22, 2014, 4:46 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/consumer/PartitionAssignor.scala, line 75 > > > > > > It seems that (a) implies (b). Not really - within a consumer instance we need each topi

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51345 --- core/src/test/scala/unit/kafka/consumer/PartitionAssignorTest.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51344 --- core/src/main/scala/kafka/consumer/ConsumerConfig.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51182 --- core/src/main/scala/kafka/consumer/PartitionAssignor.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51153 --- core/src/main/scala/kafka/consumer/PartitionAssignor.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/ --- (Updated Aug. 21, 2014, 1:10 a.m.) Review request for kafka. Bugs: KAFKA-687

Re: Review Request 23655: Patch for KAFKA-687

> On Aug. 20, 2014, 3:23 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/consumer/PartitionAllocator.scala, line 27 > > > > > > Could we add a comment on what the return value is, especially the > > String part? Su

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51104 --- core/src/main/scala/kafka/consumer/PartitionAllocator.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review51019 --- core/src/main/scala/kafka/consumer/ConsumerConfig.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/ --- (Updated Aug. 19, 2014, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-687

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review48454 --- core/src/main/scala/kafka/consumer/ConsumerConfig.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review48165 --- core/src/main/scala/kafka/consumer/ConsumerConfig.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review48319 --- core/src/main/scala/kafka/consumer/PartitionAllocator.scala

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/ --- (Updated July 18, 2014, 10:57 p.m.) Review request for kafka. Bugs: KAFKA-687

Re: Review Request 23655: Patch for KAFKA-687

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/ --- (Updated July 18, 2014, 10:55 p.m.) Review request for kafka. Summary (update