You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Grigoriy Rozhkov <gr...@gmail.com> on 2016/02/03 11:53:08 UTC

Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

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

(Updated Feb. 3, 2016, 10:53 a.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Summary (updated)
-----------------

Kafka Source support for new Consumer API FLUME-2821


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs
-----

  flume-ng-doc/sphinx/FlumeUserGuide.rst 897a2ca14b485976bfe11d413423e1c8bb789745 
  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> On March 23, 2016, 1:35 p.m., Jeff Holoman wrote:
> > flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java, line 343
> > <https://reviews.apache.org/r/41702/diff/6/?file=1283150#file1283150line343>
> >
> >     we are calling wakeup() here, do we need to catch and close in the consumer poll loop?

If I understand your question correctly, we don't need it.
Even if poll() is being invoked when we call wakeup() the poll() will throw WakeupException but not wakeup(). And consumer is correctly closed after wakeup() has been called.


- Grigoriy


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


On March 7, 2016, 4:46 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 4:46 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 897a2ca14b485976bfe11d413423e1c8bb789745 
>   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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> On March 23, 2016, 1:35 p.m., Jeff Holoman wrote:
> > pom.xml, line 53
> > <https://reviews.apache.org/r/41702/diff/6/?file=1283157#file1283157line53>
> >
> >     Lets go ahead and bump this to 9.0.1

Ok, I aggree!


- Grigoriy


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


On March 7, 2016, 4:46 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 4:46 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 897a2ca14b485976bfe11d413423e1c8bb789745 
>   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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Jeff Holoman <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125018
-----------------------------------------------------------



Looks pretty good. I agree with Tristan's comment about the translateOldProps thing. I ended up doing that at the encouragement of Jarcec so that old configs would continue to work. I have examples in the latest sink and channel patches. Thanks very much for working on this Grigoriy!!!


flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 294)
<https://reviews.apache.org/r/41702/#comment187776>

    we are calling wakeup() here, do we need to catch and close in the consumer poll loop?



pom.xml (line 53)
<https://reviews.apache.org/r/41702/#comment187767>

    Lets go ahead and bump this to 9.0.1


Loo

- Jeff Holoman


On March 7, 2016, 4:46 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 4:46 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 897a2ca14b485976bfe11d413423e1c8bb789745 
>   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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125060
-----------------------------------------------------------



Thanks for making the changes. Minor point from me but zookeeperConnect is not so much deprecated as no-longer supported. I guess this is where the new Source is different from the new Sink - as the new Source will not work using just the old configuration as the new Consumer is different in that respect.

- Tristan Stevens


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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> On March 25, 2016, 8:06 p.m., Ashish Singh wrote:
> > flume-ng-doc/sphinx/FlumeUserGuide.rst, lines 1257-1260
> > <https://reviews.apache.org/r/41702/diff/8-10/?file=1312014#file1312014line1257>
> >
> >     Won't this require consumer group id?

No, the default value 'flume' is used.


> On March 25, 2016, 8:06 p.m., Ashish Singh wrote:
> > flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java, line 447
> > <https://reviews.apache.org/r/41702/diff/8-10/?file=1312021#file1312021line447>
> >
> >     Is it possible to avoid this type casting?

I have rewritten this in another way, please let me know if you see a better solution.


- Grigoriy


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


On March 24, 2016, 6:26 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 6:26 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125458
-----------------------------------------------------------




flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1207)
<https://reviews.apache.org/r/41702/#comment188244>

    Typo in description?



flume-ng-doc/sphinx/FlumeUserGuide.rst (lines 1257 - 1260)
<https://reviews.apache.org/r/41702/#comment188245>

    Won't this require consumer group id?



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 40)
<https://reviews.apache.org/r/41702/#comment188249>

    localhost's ip can be obtained from InetAddress.getLocalHost().getHostAddress().



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java (line 447)
<https://reviews.apache.org/r/41702/#comment188251>

    Is it possible to avoid this type casting?



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java (line 474)
<https://reviews.apache.org/r/41702/#comment188250>

    Maybe add a test to verify that regex is given priority over topics list for subscribing.


- Ashish Singh


On March 24, 2016, 6:26 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 6:26 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125703
-----------------------------------------------------------




flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 46)
<https://reviews.apache.org/r/41702/#comment188571>

    No, we should never do this. Please use Runtime exception instead.


- Ashish Singh


On March 28, 2016, 5:42 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 5:42 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Alexander Alten-Lorenz <al...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review142741
-----------------------------------------------------------


Ship it!




Ship It!

- Alexander Alten-Lorenz


On March 28, 2016, 7:02 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 7:02 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125723
-----------------------------------------------------------


Ship it!




Ship It!

- Ashish Singh


On March 28, 2016, 7:02 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 7:02 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 28, 2016, 7:02 p.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Changes
-------

Throw RuntimeException in case when host address cannot be obtained


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs (updated)
-----

  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125702
-----------------------------------------------------------




flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 45)
<https://reviews.apache.org/r/41702/#comment188570>

    Sorry, I am probably not following this, but how do you stop test execution? Do you mean to add a Runtime exception here?


- Ashish Singh


On March 28, 2016, 5:42 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 5:42 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 28, 2016, 5:42 p.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Changes
-------

Stop JVM when cannot obtain HOST


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs (updated)
-----

  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> On March 28, 2016, 4:25 p.m., Ashish Singh wrote:
> > flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java, line 45
> > <https://reviews.apache.org/r/41702/diff/10-11/?file=1314117#file1314117line45>
> >
> >     If we can not get host, I do not think we can continue with the tests. We should throw an exception here.

We cannot throw Eception from a static block, so I decided to stop tests execution.


- Grigoriy


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


On March 28, 2016, 5:42 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 5:42 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125667
-----------------------------------------------------------




flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 45)
<https://reviews.apache.org/r/41702/#comment188535>

    If we can not get host, I do not think we can continue with the tests. We should throw an exception here.


- Ashish Singh


On March 28, 2016, 9:38 a.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 9:38 a.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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.

> On March 28, 2016, 4:26 p.m., Ashish Singh wrote:
> > Ship It!

Just one minor comment and we are good to go. Thanks for the awesome work Grigoriy!


- Ashish


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


On March 28, 2016, 9:38 a.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 9:38 a.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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125668
-----------------------------------------------------------


Ship it!




Ship It!

- Ashish Singh


On March 28, 2016, 9:38 a.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 28, 2016, 9:38 a.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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 28, 2016, 9:38 a.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Changes
-------

Added test that checks priority of kafka.topics.regex. Fixed typo in User Guide. Changed hard-coded host name. Avoid explicit type casting in tests.


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs (updated)
-----

  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 24, 2016, 6:26 p.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Changes
-------

Fixed typo in example for regex topic subscription.


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs (updated)
-----

  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 24, 2016, 6:22 p.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Changes
-------

Added support for regex topic subsctiption. Resolved issues commented by @Ashish Singh.


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs (updated)
-----

  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> 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 319
> > <https://reviews.apache.org/r/41702/diff/8/?file=1312016#file1312016line319>
> >
> >     group id is marked as reqd. How can we get here?

Since it has a default value I will remove it from required props and I will add a default value in log message


- 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> 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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review125122
-----------------------------------------------------------




flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1202)
<https://reviews.apache.org/r/41702/#comment187859>

    Description needs to be updated accordingly.



flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1205)
<https://reviews.apache.org/r/41702/#comment187860>

    Why don't we support regex subscription?



flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1223)
<https://reviews.apache.org/r/41702/#comment187862>

    Can we add some more details here on how we handle failures, etc?



flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1247)
<https://reviews.apache.org/r/41702/#comment187864>

    If we decide to add regex subscription support, we will have to add that as an example as well.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 56)
<https://reviews.apache.org/r/41702/#comment187865>

    May be mention that it is comma separated. Give an example.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 85)
<https://reviews.apache.org/r/41702/#comment187881>

    NIT: rename this to something like tpAndOffsetMetadata.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (lines 111 - 113)
<https://reviews.apache.org/r/41702/#comment187875>

    Really liked the way you have used nanoTime and currentTimeMillis, this is indeed the right way to use them AFAIK.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 113)
<https://reviews.apache.org/r/41702/#comment187872>

    NIT: may be rename batchEndTime to maxBatchEndTime



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 143)
<https://reviews.apache.org/r/41702/#comment187873>

    it can still have no next, right?



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (lines 144 - 145)
<https://reviews.apache.org/r/41702/#comment187874>

    NIT: inverse the order, seems counter-intuitive.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (lines 157 - 158)
<https://reviews.apache.org/r/41702/#comment187876>

    NIT: Can we combine these two lines?



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 185)
<https://reviews.apache.org/r/41702/#comment187880>

    How is lock helping here? I think checking if toBeCommitted is empty or not before committing is a better way.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 211)
<https://reviews.apache.org/r/41702/#comment187882>

    this should be kafka.consumer, right?



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 213)
<https://reviews.apache.org/r/41702/#comment187883>

    Did not make sense to me. Probably reword?



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 226)
<https://reviews.apache.org/r/41702/#comment187884>

    File a JIRA, and add JIRA id here to track it.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (lines 229 - 234)
<https://reviews.apache.org/r/41702/#comment187885>

    Make sure there are unit tests for these cases.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 283)
<https://reviews.apache.org/r/41702/#comment187886>

    group id is marked as reqd. How can we get here?



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 314)
<https://reviews.apache.org/r/41702/#comment187887>

    Does this catch even make sense now? I doubt.



flume-ng-sources/flume-kafka-source/src/main/java/org/apache/flume/source/kafka/KafkaSource.java (line 333)
<https://reviews.apache.org/r/41702/#comment187888>

    I think we can do without locks here. Please convince me otherwise.



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 77)
<https://reviews.apache.org/r/41702/#comment187890>

    Avoid using hard coded localhost address. This can be obtained from inetaddress.getlocalhost().



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 82)
<https://reviews.apache.org/r/41702/#comment187891>

    Same as above, avoid using hard coded localhost.



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java (line 114)
<https://reviews.apache.org/r/41702/#comment187893>

    Same here.



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedZookeeper.java (line 53)
<https://reviews.apache.org/r/41702/#comment187894>

    Same comment for address.



flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java (line 89)
<https://reviews.apache.org/r/41702/#comment187895>

    Can we break this into multiple smaller tests? Would it make sense to do so?


- Ashish Singh


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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-----

  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 23, 2016, 4:48 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 (updated)
-----

  flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java 7b2de7c78f15a4ce31f361f8ef8105af7b6e1e27 
  flume-ng-core/src/main/java/org/apache/flume/source/SyslogParser.java b61f7459ec9e2ccae1ec6a010385dc66598c6193 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestChannelProcessor.java c2a57487c4785209519774ad1ee6b22b567fa553 
  flume-ng-core/src/test/java/org/apache/flume/serialization/SyslogAvroEventSerializer.java d1cbcae5ff2a0aaea3807e553c1409c768d6f3d1 
  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review124759
-----------------------------------------------------------



Hi, I'm just testing this today using the configuration I used for http://blog.cloudera.com/blog/2016/03/building-benchmarking-and-tuning-syslog-ingest-architecture/.

One thing I have noticed so far is that the old configuration files don't work anymore. Perhaps you could include a translation from old to new as Jeff has done at:
https://reviews.apache.org/r/41626/diff/2-3/?collapse=1

Also/similarly, there's the line:

    if(topics == null) {
      throw new ConfigurationException("At least one Kafka topic must be specified.");
    }
    
Perhaps in this case it could fall back to looking for just a single topic specified in the old way.

- Tristan Stevens


On March 7, 2016, 4:46 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 4:46 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 897a2ca14b485976bfe11d413423e1c8bb789745 
>   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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/
-----------------------------------------------------------

(Updated March 7, 2016, 4:46 p.m.)


Review request for Flume, Grant Henke, Jarek Cecho, and Jeff Holoman.


Changes
-------

Clear headers map before using it for every event.


Repository: flume-git


Description
-------

Kafka Source support for kafka 0.9


Diffs (updated)
-----

  flume-ng-doc/sphinx/FlumeUserGuide.rst 897a2ca14b485976bfe11d413423e1c8bb789745 
  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


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Grigoriy Rozhkov <gr...@gmail.com>.

> On March 7, 2016, 4:10 p.m., Tristan Stevens wrote:
> > One minor comment from me: In KafkaSource.java it seems that the headers map isn't reset between events which could lead to leakage (especially for optional fields).

Fixed. Thank you. Could I create a pull-request?


- Grigoriy


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


On March 7, 2016, 4:46 p.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 4:46 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 897a2ca14b485976bfe11d413423e1c8bb789745 
>   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
> 
>


Re: Review Request 41702: Kafka Source support for new Consumer API FLUME-2821

Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41702/#review122304
-----------------------------------------------------------



One minor comment from me: In KafkaSource.java it seems that the headers map isn't reset between events which could lead to leakage (especially for optional fields).

- Tristan Stevens


On Feb. 3, 2016, 10:53 a.m., Grigoriy Rozhkov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41702/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2016, 10:53 a.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 897a2ca14b485976bfe11d413423e1c8bb789745 
>   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
> 
>