> On March 23, 2016, 9:49 p.m., Ashish Singh wrote:
> > flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java,
> >  lines 260-265
> > <https://reviews.apache.org/r/41702/diff/8/?file=1312016#file1312016line260>
> >
> >     Make sure there are unit tests for these cases.

Yes, it is covered in testOldProperties()


> On March 23, 2016, 9:49 p.m., Ashish Singh wrote:
> > flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java,
> >  line 162
> > <https://reviews.apache.org/r/41702/diff/8/?file=1312016#file1312016line162>
> >
> >     it can still have no next, right?

No, here the next record always exists.


> On March 23, 2016, 9:49 p.m., Ashish Singh wrote:
> > flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java,
> >  line 102
> > <https://reviews.apache.org/r/41702/diff/8/?file=1312021#file1312021line102>
> >
> >     Can we break this into multiple smaller tests? Would it make sense to 
> > do so?

This test is highly cohesive even if it does not looks like so. I think we can 
leave it as it is.


- Grigoriy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125122
-----------------------------------------------------------


On March 23, 2016, 4:59 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 4:59 p.m.)
> 
> 
> Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Kafka Source support for kafka 0.9
> 
> 
> Diffs
> -----
> 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst 
> 423e0cf8a3fffbdab6363f4f71df7fd8c3c133a3 
>   flume-ng-sources/flume-kafka-source/pom.xml 
> 0f93476c61e0281d45a15426493c4c3579503cee 
>   
> flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java
>  fd1dd3c17bda875daaecef02cd7c537bb14242b8 
>   
> flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSourceConstants.java
>  911012cefcd656bac3308c3e02990a6ff42a0de5 
>   
> flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSourceUtil.java
>  4a4034bd826f67e790e16b67e5b52f469b182627 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java
>  26c5c9d0aa29fbbc1d9eced70b26aa3f81855c26 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedZookeeper.java
>  1b8a27106e84b3ce1d8a100d31cf33d847c68f1b 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java
>  8ec14cccf555ab38449b1bcb1e41a6ecbd19fe7c 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSourceUtil.java
>  0cbb4b69eca3b052a4b74a52a71ea3e45ce5f814 
>   pom.xml 15c086b5090305a1a70185626452a05cb7c1941d 
> 
> Diff: https://reviews.apache.org/r/41702/diff/
> 
> 
> Testing
> -------
> 
> New features:
> - multiple topic subscription
> Style for configuring new Kafka Source is made following styles in these 
> patches.
> https://reviews.apache.org/r/41626/
> https://reviews.apache.org/r/41629/
> 
> generate command:
> git diff --no-prefix --full-index > FLUME-2821.patch
> 
> apply command:
> patch -p0 < FLUME-2821.patch
> 
> 
> Thanks,
> 
> Grigoriy Rozhkov
> 
>

Reply via email to