Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review80607 --- There seems to be a bunch of checkstyle failures. Could you address those? ./gradlew -Dtest.single=SerializationTest cleanTest client:test [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:17:8: Unused import - java.nio.ByteBuffer. [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:27: if at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:28: if child at indentation level 8 not at correct indentation, 12 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:29: if at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:30: if child at indentation level 8 not at correct indentation, 12 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:32: if rcurly at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:34: method def child at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:35: for at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:36: for child at indentation level 8 not at correct indentation, 12 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:37: for child at indentation level 8 not at correct indentation, 12 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:37:15: Unnecessary parentheses around assignment right-hand side. [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:38: for rcurly at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java:39: method def child at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:24: if at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:25: if child at indentation level 8 not at correct indentation, 12 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:27: method def child at indentation level 6 not at correct indentation, 8 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:28: array initialization child at indentation level 8 not at correct indentation, 10 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:29: array initialization child at indentation level 8 not at correct indentation, 10 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:30: array initialization child at indentation level 8 not at correct indentation, 10 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:31: method call child at indentation level 8 not at correct indentation, 10 [ant:checkstyle] /Users/junrao/intellij/kafka/clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java:35: method def modifier at indentation level 2 not at correct indentation, 4 :clients:checkstyleMain FAILED - Jun Rao On April 18, 2015, midnight, Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 18, 2015, midnight) Review request for kafka. Bugs:
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 17, 2015, 9:49 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 17, 2015, 10:09 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On April 7, 2015, 10:41 p.m., Jun Rao wrote: Sorry for the late review. A few more comments below. Done! - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review77523 --- On April 17, 2015, 10:09 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 17, 2015, 10:09 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On March 25, 2015, 4:48 p.m., Mayuresh Gharat wrote: Thanks for the review. Addressed your comment. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review77747 --- On April 17, 2015, 10:09 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 17, 2015, 10:09 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review80555 --- Thanks for the patch. A few more minor comments. clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java https://reviews.apache.org/r/31369/#comment130586 Perhaps we can put b 0xFF in brackets to make the precedence clear? clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java https://reviews.apache.org/r/31369/#comment130579 Perhaps we can combine the two tests. We don't need to test null twice. Instead of deplicating the test code for positive and negative values, we can probably just to put the test code in a loop and iterate it twice. examples/src/main/java/kafka/examples/Producer.java https://reviews.apache.org/r/31369/#comment130583 record - message - Jun Rao On April 17, 2015, 10:09 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 17, 2015, 10:09 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 18, 2015, midnight) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On April 17, 2015, 11:05 p.m., Jun Rao wrote: Thanks for the patch. A few more minor comments. Jun thanks for the review again. Addressed your comments. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review80555 --- On April 18, 2015, midnight, Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated April 18, 2015, midnight) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31369/#review77523
---
Sorry for the late review. A few more comments below.
clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java
https://reviews.apache.org/r/31369/#comment125614
We probably should throw a SerializationException if the size of the data
is not 4.
clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java
https://reviews.apache.org/r/31369/#comment128503
ByteBuffer.array() returns the backing array. The actual content may not
start at offset 0.
Could we do sth like the following and do the reverse of that in
deserialization?
return new byte[] {
(byte)(value 24),
(byte)(value 16),
(byte)(value 8),
(byte)value};
clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java
https://reviews.apache.org/r/31369/#comment128504
Perhaps we can test a negative value too?
examples/src/main/java/kafka/examples/Producer.java
https://reviews.apache.org/r/31369/#comment125615
Could we make the output here consistent with line 62? Probably use the
format record(key, value)?
- Jun Rao
On March 4, 2015, 1:51 a.m., Ashish Singh wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31369/
---
(Updated March 4, 2015, 1:51 a.m.)
Review request for kafka.
Bugs: KAFKA-1982
https://issues.apache.org/jira/browse/KAFKA-1982
Repository: kafka
Description
---
KAFKA-1982: change kafka.examples.Producer to use the new java producer
Diffs
-
clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java
PRE-CREATION
clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java
PRE-CREATION
clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java
f5cd61c1aa9433524da0b83826a766389de68a0b
examples/README 53db6969b2e2d49e23ab13283b9146848e37434e
examples/src/main/java/kafka/examples/Consumer.java
13135b954f3078eeb7394822b0db25470b746f03
examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java
1239394190fe557e025fbd8f3803334402b0aeea
examples/src/main/java/kafka/examples/Producer.java
96e98933148d07564c1b30ba8e805e2433c2adc8
examples/src/main/java/kafka/examples/SimpleConsumerDemo.java
0d66fe5f8819194c8624aed4a21105733c20cc8e
Diff: https://reviews.apache.org/r/31369/diff/
Testing
---
Thanks,
Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review75529 --- Ship it! Thats a really sweet producer example :) LGTM. - Gwen Shapira On March 4, 2015, 1:51 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated March 4, 2015, 1:51 a.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated March 4, 2015, 1:51 a.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On March 3, 2015, 5:42 a.m., Jun Rao wrote: Thanks for the review Jun! Addressed your concerns. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review74895 --- On March 4, 2015, 1:51 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated March 4, 2015, 1:51 a.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On Feb. 27, 2015, 7:29 p.m., Gwen Shapira wrote: Thanks for the patch, Ashish. Its shaping up to be a very useful example. Two comments: 1. I think the ser/de should be part of the example and not in common, I'm not sure integer ser/de is useful enough to be distributed with Kafka (although Jun can correct me if I got this wrong). 2. I saw a lot of discussion on the mailing list around using the new producer async vs. sync. This example shows the async path. Do we want to add another sync example where we do something like: val future = producer.send(new ProducerRecordInteger, String(topic, messageNo, messageStr), new DemoCallBack(startTime, messageNo, messageStr)); // this waits for send to complete future.get Jun Rao wrote: Gwen, Integer may be a common type for keys. So, it probably makes sense to include Integer ser/de in common. I agree that it would be useful to add a sync example. Added sync example. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review74553 --- On March 4, 2015, 1:51 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated March 4, 2015, 1:51 a.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java f5cd61c1aa9433524da0b83826a766389de68a0b examples/README 53db6969b2e2d49e23ab13283b9146848e37434e examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/KafkaConsumerProducerDemo.java 1239394190fe557e025fbd8f3803334402b0aeea examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 examples/src/main/java/kafka/examples/SimpleConsumerDemo.java 0d66fe5f8819194c8624aed4a21105733c20cc8e Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review74895 --- clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java https://reviews.apache.org/r/31369/#comment121727 Could we add a unit test for Integer Ser/DeSer? clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java https://reviews.apache.org/r/31369/#comment121724 Incorrect comment. clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java https://reviews.apache.org/r/31369/#comment121722 Incorrect comment. examples/src/main/java/kafka/examples/Producer.java https://reviews.apache.org/r/31369/#comment121726 We should handle the case when metadata is null. - Jun Rao On Feb. 27, 2015, 7:08 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 27, 2015, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On Feb. 27, 2015, 7:29 p.m., Gwen Shapira wrote: Thanks for the patch, Ashish. Its shaping up to be a very useful example. Two comments: 1. I think the ser/de should be part of the example and not in common, I'm not sure integer ser/de is useful enough to be distributed with Kafka (although Jun can correct me if I got this wrong). 2. I saw a lot of discussion on the mailing list around using the new producer async vs. sync. This example shows the async path. Do we want to add another sync example where we do something like: val future = producer.send(new ProducerRecordInteger, String(topic, messageNo, messageStr), new DemoCallBack(startTime, messageNo, messageStr)); // this waits for send to complete future.get Gwen, Integer may be a common type for keys. So, it probably makes sense to include Integer ser/de in common. I agree that it would be useful to add a sync example. - Jun --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review74553 --- On Feb. 27, 2015, 7:08 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 27, 2015, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 27, 2015, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review74553 --- Thanks for the patch, Ashish. Its shaping up to be a very useful example. Two comments: 1. I think the ser/de should be part of the example and not in common, I'm not sure integer ser/de is useful enough to be distributed with Kafka (although Jun can correct me if I got this wrong). 2. I saw a lot of discussion on the mailing list around using the new producer async vs. sync. This example shows the async path. Do we want to add another sync example where we do something like: val future = producer.send(new ProducerRecordInteger, String(topic, messageNo, messageStr), new DemoCallBack(startTime, messageNo, messageStr)); // this waits for send to complete future.get - Gwen Shapira On Feb. 27, 2015, 7:08 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 27, 2015, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
On Feb. 26, 2015, 10:27 p.m., Jun Rao wrote: examples/src/main/java/kafka/examples/Consumer.java, line 62 https://reviews.apache.org/r/31369/diff/4/?file=875219#file875219line62 It would be useful to print out the key as well. Added On Feb. 26, 2015, 10:27 p.m., Jun Rao wrote: examples/src/main/java/kafka/examples/Producer.java, line 53 https://reviews.apache.org/r/31369/diff/4/?file=875220#file875220line53 Perhaps we can create an IntegerSerializer and send messageNo as the key? Done On Feb. 26, 2015, 10:27 p.m., Jun Rao wrote: examples/src/main/java/kafka/examples/Producer.java, lines 81-88 https://reviews.apache.org/r/31369/diff/4/?file=875220#file875220line81 It's probably better to print each mesage in a single line and print out the key as well. Done - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/#review74379 --- On Feb. 27, 2015, 7:08 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 27, 2015, 7:08 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/IntegerDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/IntegerSerializer.java PRE-CREATION examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 24, 2015, 6:35 p.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 25, 2015, 4:48 a.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
Re: Review Request 31369: Patch for KAFKA-1982
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31369/ --- (Updated Feb. 25, 2015, 4:45 a.m.) Review request for kafka. Bugs: KAFKA-1982 https://issues.apache.org/jira/browse/KAFKA-1982 Repository: kafka Description --- KAFKA-1982: change kafka.examples.Producer to use the new java producer Diffs (updated) - examples/src/main/java/kafka/examples/Consumer.java 13135b954f3078eeb7394822b0db25470b746f03 examples/src/main/java/kafka/examples/Producer.java 96e98933148d07564c1b30ba8e805e2433c2adc8 Diff: https://reviews.apache.org/r/31369/diff/ Testing --- Thanks, Ashish Singh
