You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Rahul Ravindran <ra...@yahoo.com> on 2012/11/26 03:23:50 UTC

Review Request: FLUME-1713 Netcat source to not return "OK"

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

Review request for Flume.


Description
-------

FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test


This addresses bug FLUME-1713.
    https://issues.apache.org/jira/browse/FLUME-1713


Diffs
-----

  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
  flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 

Diff: https://reviews.apache.org/r/8220/diff/


Testing
-------

Unit test added


Thanks,

Rahul Ravindran

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Rahul Ravindran <ra...@yahoo.com>.

> On Nov. 28, 2012, 2:54 a.m., Mike Percy wrote:
> > Looks good!
> > 
> > Please make the following changes and then attach the patch to the JIRA:
> > * Rename ackEveryEvent to ack-every-event to remain consistent with the existing elements. Even though we have agreed to make all new properties camel caps, in the case of existing components we should remain consistent with the existing stuff
> > * Please document this new parameter in the user guide @ flume-ng-doc/sphinx/FlumeUserGuide.rst

Thanks Mike!

I am assuming the rename (of ackEveryEvent to act-every-event) is only for the constant which is used by users to set the flag in the config(not in the variable names in code). So, new parameter in the document in FlumeUserGuide.rst would be ack-every-event. Could you confirm?


- Rahul


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


On Nov. 26, 2012, 2:24 a.m., Rahul Ravindran wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8220/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 2:24 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test
> 
> 
> This addresses bug FLUME-1713.
>     https://issues.apache.org/jira/browse/FLUME-1713
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
>   flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 
> 
> Diff: https://reviews.apache.org/r/8220/diff/
> 
> 
> Testing
> -------
> 
> Unit test added
> 
> 
> Thanks,
> 
> Rahul Ravindran
> 
>

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Mike Percy <mp...@apache.org>.

> On Nov. 28, 2012, 2:54 a.m., Mike Percy wrote:
> > Looks good!
> > 
> > Please make the following changes and then attach the patch to the JIRA:
> > * Rename ackEveryEvent to ack-every-event to remain consistent with the existing elements. Even though we have agreed to make all new properties camel caps, in the case of existing components we should remain consistent with the existing stuff
> > * Please document this new parameter in the user guide @ flume-ng-doc/sphinx/FlumeUserGuide.rst
> 
> Rahul Ravindran wrote:
>     Thanks Mike!
>     
>     I am assuming the rename (of ackEveryEvent to act-every-event) is only for the constant which is used by users to set the flag in the config(not in the variable names in code). So, new parameter in the document in FlumeUserGuide.rst would be ack-every-event. Could you confirm?

Sure thing. Yes this is just the user facing property they would put in their flume config file. Not the Java source code. This is for consistency with max-line-length (otherwise I would agree with camel caps as that has already been agreed upon). Thank you sir!


- Mike


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


On Nov. 26, 2012, 2:24 a.m., Rahul Ravindran wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8220/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 2:24 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test
> 
> 
> This addresses bug FLUME-1713.
>     https://issues.apache.org/jira/browse/FLUME-1713
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
>   flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 
> 
> Diff: https://reviews.apache.org/r/8220/diff/
> 
> 
> Testing
> -------
> 
> Unit test added
> 
> 
> Thanks,
> 
> Rahul Ravindran
> 
>

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8220/#review13797
-----------------------------------------------------------

Ship it!


Looks good!

Please make the following changes and then attach the patch to the JIRA:
* Rename ackEveryEvent to ack-every-event to remain consistent with the existing elements. Even though we have agreed to make all new properties camel caps, in the case of existing components we should remain consistent with the existing stuff
* Please document this new parameter in the user guide @ flume-ng-doc/sphinx/FlumeUserGuide.rst

- Mike Percy


On Nov. 26, 2012, 2:24 a.m., Rahul Ravindran wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8220/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 2:24 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test
> 
> 
> This addresses bug FLUME-1713.
>     https://issues.apache.org/jira/browse/FLUME-1713
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
>   flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 
> 
> Diff: https://reviews.apache.org/r/8220/diff/
> 
> 
> Testing
> -------
> 
> Unit test added
> 
> 
> Thanks,
> 
> Rahul Ravindran
> 
>

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8220/#review13845
-----------------------------------------------------------

Ship it!


Ship It!

- Mike Percy


On Nov. 28, 2012, 9:02 p.m., Rahul Ravindran wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8220/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2012, 9:02 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test
> 
> 
> This addresses bug FLUME-1713.
>     https://issues.apache.org/jira/browse/FLUME-1713
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
>   flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst b4a8868 
>   flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 
> 
> Diff: https://reviews.apache.org/r/8220/diff/
> 
> 
> Testing
> -------
> 
> Unit test added
> 
> 
> Thanks,
> 
> Rahul Ravindran
> 
>

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Rahul Ravindran <ra...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8220/
-----------------------------------------------------------

(Updated Nov. 28, 2012, 9:02 p.m.)


Review request for Flume.


Changes
-------

Final patch after incorporating Mike's comments


Description
-------

FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test


This addresses bug FLUME-1713.
    https://issues.apache.org/jira/browse/FLUME-1713


Diffs (updated)
-----

  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
  flume-ng-doc/sphinx/FlumeUserGuide.rst b4a8868 
  flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 

Diff: https://reviews.apache.org/r/8220/diff/


Testing
-------

Unit test added


Thanks,

Rahul Ravindran

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Rahul Ravindran <ra...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8220/
-----------------------------------------------------------

(Updated Nov. 26, 2012, 2:24 a.m.)


Review request for Flume.


Description
-------

FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test


This addresses bug FLUME-1713.
    https://issues.apache.org/jira/browse/FLUME-1713


Diffs
-----

  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
  flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 

Diff: https://reviews.apache.org/r/8220/diff/


Testing
-------

Unit test added


Thanks,

Rahul Ravindran

Re: Review Request: FLUME-1713 Netcat source to not return "OK"

Posted by Rahul Ravindran <ra...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8220/
-----------------------------------------------------------

(Updated Nov. 26, 2012, 2:24 a.m.)


Review request for Flume.


Description
-------

FLUME-1713 Netcat source should allow for *not* returning OK upon receipt of each message; Added a config boolean parameter for this source ackEveryEvent(default value:false). Add parameterized test to existing Netcat unit test


This addresses bug FLUME-1713.
    https://issues.apache.org/jira/browse/FLUME-1713


Diffs
-----

  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java 37c09fe 
  flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java 1d8b5e4 
  flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 3c17d3d 

Diff: https://reviews.apache.org/r/8220/diff/


Testing
-------

Unit test added


Thanks,

Rahul Ravindran