You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Tristan Stevens <tr...@cloudera.com> on 2016/10/03 17:51:13 UTC
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/
-----------------------------------------------------------
(Updated Oct. 3, 2016, 5:51 p.m.)
Review request for Flume and Bal�zs Don�t Bessenyei.
Changes
-------
Updated based on feedback.
Repository: flume-git
Description
-------
Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
Diffs (updated)
-----
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
Diff: https://reviews.apache.org/r/52279/diff/
Testing
-------
Unit tests provided.
Thanks,
Tristan Stevens
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Balázs Donát Bessenyei <be...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/#review166164
-----------------------------------------------------------
I was using https://gist.github.com/bessbd/8d5c07405fd8408f9db4673de291db92 as configuration and ```nc -u 127.0.0.1 6666``` to feed some input manually.
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java (line 76)
<https://reviews.apache.org/r/52279/#comment238057>
Is there a way to rewrite this without mentioning syslog?
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java (line 120)
<https://reviews.apache.org/r/52279/#comment238058>
Is there a way to rewrite this without mentioning syslog?
flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1381)
<https://reviews.apache.org/r/52279/#comment238053>
I think this should be netcatudp
Also, for this to work I needed to add
```
/**
* Netcat UDP source.
*
* @see NetcatSource
*/
NETCATUDP("org.apache.flume.conf.source.NetcatUdpSourceConfiguration"),
```
to flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java
flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1383)
<https://reviews.apache.org/r/52279/#comment238051>
Did you mean r1.port instead of r1.bind?
- Bal�zs Don�t Bessenyei
On Oct. 4, 2016, 11:30 a.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52279/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2016, 11:30 a.m.)
>
>
> Review request for Flume and Bal�zs Don�t Bessenyei.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
>
>
> Diffs
> -----
>
> flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
> flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
> flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
> flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
> flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
>
> Diff: https://reviews.apache.org/r/52279/diff/
>
>
> Testing
> -------
>
> Unit tests provided.
>
>
> Thanks,
>
> Tristan Stevens
>
>
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/
-----------------------------------------------------------
(Updated June 26, 2017, 6:43 p.m.)
Review request for Flume and Balázs Donát Bessenyei.
Changes
-------
Fix final defect
Repository: flume-git
Description
-------
Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
Diffs (updated)
-----
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java 6bd14896
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a6
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa85003
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
flume-ng-doc/sphinx/FlumeUserGuide.rst 2073bf63
Diff: https://reviews.apache.org/r/52279/diff/7/
Changes: https://reviews.apache.org/r/52279/diff/6-7/
Testing
-------
Unit tests provided.
Thanks,
Tristan Stevens
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Balázs Donát Bessenyei <be...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/#review168996
-----------------------------------------------------------
Fix it, then Ship it!
Other than the SourceConfiguration change, LGTM
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java
Lines 224 (patched)
<https://reviews.apache.org/r/52279/#comment241324>
For me, this only works with
NETCATUDP("org.apache.flume.conf.source.NetcatUdpSourceConfiguration"),
- Bal�zs Don�t Bessenyei
On Feb. 22, 2017, 11:10 p.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52279/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 11:10 p.m.)
>
>
> Review request for Flume and Bal�zs Don�t Bessenyei.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
>
>
> Diffs
> -----
>
> flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java 6bd1489
> flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
> flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
> flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
> flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
> flume-ng-doc/sphinx/FlumeUserGuide.rst d863068
>
>
> Diff: https://reviews.apache.org/r/52279/diff/6/
>
>
> Testing
> -------
>
> Unit tests provided.
>
>
> Thanks,
>
> Tristan Stevens
>
>
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/
-----------------------------------------------------------
(Updated Feb. 22, 2017, 11:10 p.m.)
Review request for Flume and Bal�zs Don�t Bessenyei.
Changes
-------
Updated following bessbd's comments and rebased against trunk.
Repository: flume-git
Description
-------
Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
Diffs (updated)
-----
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java 6bd1489
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
flume-ng-doc/sphinx/FlumeUserGuide.rst d863068
Diff: https://reviews.apache.org/r/52279/diff/
Testing
-------
Unit tests provided.
Thanks,
Tristan Stevens
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/
-----------------------------------------------------------
(Updated Oct. 4, 2016, 11:30 a.m.)
Review request for Flume and Bal�zs Don�t Bessenyei.
Changes
-------
Fixed checkstyle issues.
Repository: flume-git
Description
-------
Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
Diffs (updated)
-----
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
Diff: https://reviews.apache.org/r/52279/diff/
Testing
-------
Unit tests provided.
Thanks,
Tristan Stevens
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/
-----------------------------------------------------------
(Updated Oct. 4, 2016, 10:46 a.m.)
Review request for Flume and Bal�zs Don�t Bessenyei.
Repository: flume-git
Description
-------
Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
Diffs (updated)
-----
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
Diff: https://reviews.apache.org/r/52279/diff/
Testing
-------
Unit tests provided.
Thanks,
Tristan Stevens
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Balázs Donát Bessenyei <be...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/#review151304
-----------------------------------------------------------
Checkstyle still reports errors:
[INFO] --- maven-checkstyle-plugin:2.17:check (verify) @ flume-ng-core ---
[INFO] There are 11 errors reported by Checkstyle 6.19 with flume/checkstyle.xml ruleset.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[40] (imports) AvoidStarImport: Using the '.*' form of import should be avoided - org.jboss.netty.channel.*.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[71,78] (whitespace) WhitespaceAround: WhitespaceAround: '{' is not preceded with whitespace.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[116] (indentation) Indentation: 'catch rcurly' have incorrect indentation level 8, expected level should be 6.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[124] (indentation) Indentation: 'operator new lparen' have incorrect indentation level 8, expected level should be 4.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[124,9] (whitespace) MethodParamPad: '(' should be on the previous line.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[127] (indentation) Indentation: 'new' have incorrect indentation level 6, expected level should be 8.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[132] (indentation) Indentation: 'method def' child have incorrect indentation level 7, expected level should be one of the following: 8, 10, 12.
[ERROR] src/main/java/org/apache/flume/source/NetcatUdpSource.java:[134] (indentation) Indentation: 'object def rcurly' have incorrect indentation level 5, expected level should be one of the following: 4, 6, 8.
[ERROR] src/test/java/org/apache/flume/source/TestNetcatUdpSource.java:[42] (whitespace) EmptyLineSeparator: 'CLASS_DEF' has more than 1 empty lines before.
[ERROR] src/test/java/org/apache/flume/source/TestNetcatUdpSource.java:[44] (indentation) Indentation: 'LoggerFactory' have incorrect indentation level 4, expected level should be 6.
[ERROR] src/test/java/org/apache/flume/source/TestNetcatUdpSource.java:[49] (sizes) LineLength: Line is longer than 100 characters (found 141).
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java (line 118)
<https://reviews.apache.org/r/52279/#comment219646>
Should be *...NetcatUdpSource
- Bal�zs Don�t Bessenyei
On Oct. 4, 2016, 6:18 a.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52279/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2016, 6:18 a.m.)
>
>
> Review request for Flume and Bal�zs Don�t Bessenyei.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
>
>
> Diffs
> -----
>
> flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
> flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
> flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
> flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
> flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
>
> Diff: https://reviews.apache.org/r/52279/diff/
>
>
> Testing
> -------
>
> Unit tests provided.
>
>
> Thanks,
>
> Tristan Stevens
>
>
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Tristan Stevens <tr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/
-----------------------------------------------------------
(Updated Oct. 4, 2016, 6:18 a.m.)
Review request for Flume and Bal�zs Don�t Bessenyei.
Changes
-------
Fix typo NetcatHand(l)er
Repository: flume-git
Description
-------
Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
Diffs (updated)
-----
flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
Diff: https://reviews.apache.org/r/52279/diff/
Testing
-------
Unit tests provided.
Thanks,
Tristan Stevens
Re: Review Request 52279: FLUME-2917 Provide netcat UDP source as
alternative to TCP
Posted by Lior Zeno <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52279/#review151211
-----------------------------------------------------------
flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java (line 67)
<https://reviews.apache.org/r/52279/#comment219490>
nit: s/NetcatHander/NetcatHandler
- Lior Zeno
On Oct. 3, 2016, 5:51 p.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52279/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2016, 5:51 p.m.)
>
>
> Review request for Flume and Bal�zs Don�t Bessenyei.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Implementation is heavily based on the existing SyslogUDPSource - the tests and implementation mirror that. We could consider making it a superclass of SyslogUDPSource and removing the syslog parsing, if this was felt tidier.
>
>
> Diffs
> -----
>
> flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java 4f4073a
> flume-ng-core/src/main/java/org/apache/flume/source/NetcatUdpSource.java PRE-CREATION
> flume-ng-core/src/test/java/org/apache/flume/source/TestDefaultSourceFactory.java baa8500
> flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatUdpSource.java PRE-CREATION
> flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38
>
> Diff: https://reviews.apache.org/r/52279/diff/
>
>
> Testing
> -------
>
> Unit tests provided.
>
>
> Thanks,
>
> Tristan Stevens
>
>