Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review70291 --- Ship it! - Guozhang Wang On Jan. 29, 2015, 11:20 a.m., Jay Kreps

Re: Review Request 27799: New consumer

> On Jan. 20, 2015, 8:03 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java, > > line 114 > > > > > > Should this inherit from CommonClientConfig? > > Jay Kreps

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 29, 2015, 11:20 a.m.) Review request for kafka. Bugs: KAFKA-176

Re: Review Request 27799: New consumer

> On Jan. 28, 2015, 1:34 a.m., Guozhang Wang wrote: > > "patch -p1 < patch-file" does not do the renaming of > > RequestCompletionHandler.java so I have to do that manually (weird), but > > other than that, build / test LGTM. > > > > It seems some of previous comments are not addressed yet. F

Re: Review Request 27799: New consumer

> On Jan. 27, 2015, 10:25 a.m., Onur Karaman wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java, > > line 205 > > > > > > You left a blah. This one is actually intentional. We have

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 5:35 p.m., Aditya Auradkar wrote: > > clients/src/main/java/org/apache/kafka/clients/RequestCompletionHandler.java, > > line 21 > > > > > > nit. Can we remove the public from the interface methods?

Re: Review Request 27799: New consumer

> On Jan. 20, 2015, 8:03 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/KafkaClient.java, lines 30-33 > > > > > > Wondering why we make newline for @param but keep the same line for > > @

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review69936 --- "patch -p1 < patch-file" does not do the renaming of RequestComplet

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review69737 --- minor nitpicks clients/src/main/java/org/apache/kafka/clients/cons

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 23, 2015, 9:15 p.m.) Review request for kafka. Changes ---

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 23, 2015, 9:13 p.m.) Review request for kafka. Bugs: KAFKA-1760

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 23, 2015, 9:15 p.m.) Review request for kafka. Bugs: KAFKA-1760

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review69453 --- core/src/test/scala/integration/kafka/api/ConsumerTest.scala

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 7:10 p.m., Guozhang Wang wrote: > > core/src/test/scala/integration/kafka/api/ConsumerTest.scala, line 64 > > > > > > Shall we add the @Test label just in case? > > Jay Kreps wrote: > No I don't

Re: Review Request 27799: New consumer

> On Jan. 23, 2015, 8:57 a.m., Ewen Cheslack-Postava wrote: > > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java, line 247 > > > > > > I think these methods need to have timeouts on them. They get call

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review69355 --- clients/src/main/java/org/apache/kafka/clients/ClusterConnectionSta

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 7:10 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/utils/TestUtils.scala, lines 733-743 > > > > > > Is this the same as createTopic in line 172? > > Jay Kreps wrote: > Yeah, wei

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 23, 2015, 4:22 a.m.) Review request for kafka. Bugs: KAFKA-1760

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 7:10 p.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line > > 198 > > > > > > Just trying to understand the rationale why we want to special-care

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review68947 --- clients/src/main/java/org/apache/kafka/common/network/Selector.java

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 5:35 p.m., Aditya Auradkar wrote: > > clients/src/main/java/org/apache/kafka/clients/RequestCompletionHandler.java, > > line 21 > > > > > > nit. Can we remove the public from the interface methods?

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 5:35 p.m., Aditya Auradkar wrote: > > clients/src/main/java/org/apache/kafka/clients/RequestCompletionHandler.java, > > line 21 > > > > > > nit. Can we remove the public from the interface methods?

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 22, 2015, 6:06 p.m.) Review request for kafka. Bugs: KAFKA-1760

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 22, 2015, 6:03 p.m.) Review request for kafka. Bugs: KAFKA-1760

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 5:35 p.m., Aditya Auradkar wrote: > > clients/src/main/java/org/apache/kafka/clients/RequestCompletionHandler.java, > > line 21 > > > > > > nit. Can we remove the public from the interface methods?

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review69204 --- clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 2:45 a.m., Onur Karaman wrote: > > clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java, > > line 32 > > > > > > Other CURRENT_SCHEMA's throughout the rb were changed to be f

Re: Review Request 27799: New consumer

> On Jan. 22, 2015, 3:14 a.m., Jaikiran Pai wrote: > > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java, line 253 > > > > > > Hi Jay, > > > > I think doing this unmuteAll in a finally block mi

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/#review69121 --- clients/src/main/java/org/apache/kafka/clients/NetworkClient.java <

Re: Review Request 27799: New consumer

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

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 21, 2015, 4:47 p.m.) Review request for kafka. Bugs: KAFKA-1760

Re: Review Request 27799: New consumer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27799/ --- (Updated Jan. 21, 2015, 4:42 p.m.) Review request for kafka. Summary (updated