Flume
  1. Flume
  2. FLUME-2628

Add an optional parameter to specify the expected input text encoding for the netcat sourcef the netcat source

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: v1.5.1
    • Fix Version/s: v1.7.0
    • Component/s: Sinks+Sources
    • Labels:
    • Release Note:
      Add an optional parameter to specify the expected input text encoding for the netcat source

      Description

      Add the 'encoding' configuration parameter to specify the expected text encoding of the netcat source in input.
      This parameter is optional and defaulted to "utf-8" (same value as the one hardcoded before the patch)

      1. FLUME-2628.patch
        17 kB
        Lionel Herbet
      2. FLUME-2628.patch
        16 kB
        Lionel Herbet
      3. FLUME-2628.patch
        17 kB
        Lionel Herbet
      4. Netcat_Encoding_WithUnitTest.patch
        17 kB
        Lionel Herbet
      5. Netcat_Encoding.patch
        3 kB
        Lionel Herbet
      6. Netcat_WithUnitTest.patch
        29 kB
        Lionel Herbet

        Activity

        Hide
        Lionel Herbet added a comment -

        Patch fot the netcat source

        Show
        Lionel Herbet added a comment - Patch fot the netcat source
        Hide
        Johny Rufus added a comment -

        +1, the patch looks good, Needs a few test cases, to test the new parameter, with a few different charsets

        Show
        Johny Rufus added a comment - +1, the patch looks good, Needs a few test cases, to test the new parameter, with a few different charsets
        Hide
        Johny Rufus added a comment -

        Hi Lionel Herbet, Is it possible to add a test case as part of the TestNetcatSource file and include it in the patch.
        Typically for this scenario, it would be good to add one case where a non utf-8 encoding (utf-16 may be )can be specified for the source, and for input data encoded in that format, making sure that the Netcat source decodes it correctly

        Show
        Johny Rufus added a comment - Hi Lionel Herbet , Is it possible to add a test case as part of the TestNetcatSource file and include it in the patch. Typically for this scenario, it would be good to add one case where a non utf-8 encoding (utf-16 may be )can be specified for the source, and for input data encoded in that format, making sure that the Netcat source decodes it correctly
        Hide
        Lionel Herbet added a comment -

        I did more modification. Maybe too much.
        I added Unit Test to Netcat on more functionalities that only encoding as i did not found Unit Test for the NetcatSource.

        Here the functionalities i added :
        Add an 'encoding' configuration option to identify input encoding for the netcat source. The default value for the encoding parameter is "utf-8".
        Moreover a code refactoring was performed to use IOUtils from commons-io instead of specific code for readability purpose.
        Instead of exiting when an event exceed the maximum line length configured, this one is discarded.
        Finaly a batch option can be configured to send event in batch to the Channel

        In can revert to a subset of these functionalities. Tell me as you want.

        Show
        Lionel Herbet added a comment - I did more modification. Maybe too much. I added Unit Test to Netcat on more functionalities that only encoding as i did not found Unit Test for the NetcatSource. Here the functionalities i added : Add an 'encoding' configuration option to identify input encoding for the netcat source. The default value for the encoding parameter is "utf-8". Moreover a code refactoring was performed to use IOUtils from commons-io instead of specific code for readability purpose. Instead of exiting when an event exceed the maximum line length configured, this one is discarded. Finaly a batch option can be configured to send event in batch to the Channel In can revert to a subset of these functionalities. Tell me as you want.
        Hide
        Johny Rufus added a comment -

        Hi Lionel Herbet, thanks, can we limit the scope of the bug to the description, and track the additional refactoring , as part of a different issue.

        Show
        Johny Rufus added a comment - Hi Lionel Herbet , thanks, can we limit the scope of the bug to the description, and track the additional refactoring , as part of a different issue.
        Hide
        Lionel Herbet added a comment - - edited

        Sorry for the inconvenience, you are absolutly right.
        Here the patch withe the unitTest for the Netcat Source.

        Show
        Lionel Herbet added a comment - - edited Sorry for the inconvenience, you are absolutly right. Here the patch withe the unitTest for the Netcat Source.
        Hide
        Johny Rufus added a comment -

        thanks Lionel Herbet,
        +1 , the patch looks good

        Show
        Johny Rufus added a comment - thanks Lionel Herbet , +1 , the patch looks good
        Hide
        Johny Rufus added a comment -

        Lionel Herbet, can you rename the final patch as per the patch naming guidelines, thanks !
        https://cwiki.apache.org/confluence/display/FLUME/How+to+Contribute#HowtoContribute-ProvidingPatches
        "FLUME-2628.patch"

        Show
        Johny Rufus added a comment - Lionel Herbet , can you rename the final patch as per the patch naming guidelines, thanks ! https://cwiki.apache.org/confluence/display/FLUME/How+to+Contribute#HowtoContribute-ProvidingPatches " FLUME-2628 .patch"
        Hide
        Lionel Herbet added a comment -

        Johny Rufus, the patch with the correct naming convention.

        Show
        Lionel Herbet added a comment - Johny Rufus, the patch with the correct naming convention.
        Hide
        Johny Rufus added a comment -

        Lionel Herbet, the patch does not apply cleanly,
        can you do a "git diff"

        Show
        Johny Rufus added a comment - Lionel Herbet , the patch does not apply cleanly, can you do a "git diff"
        Hide
        Lionel Herbet added a comment -

        Same Patch but with Git diff. I hope it will work.

        Show
        Lionel Herbet added a comment - Same Patch but with Git diff. I hope it will work.
        Hide
        Johny Rufus added a comment -

        Thanks Lionel Herbet, the patch works, ran tests and everything looks good,
        one more issue to be resolved before I can commit, can you include the Apache License Header to the new file - "TestNetcatSource.java" (sorry I missed this in my review)

        Show
        Johny Rufus added a comment - Thanks Lionel Herbet , the patch works, ran tests and everything looks good, one more issue to be resolved before I can commit, can you include the Apache License Header to the new file - "TestNetcatSource.java" (sorry I missed this in my review)
        Hide
        Lionel Herbet added a comment -

        The new version with Apache header included, still in git diff.

        Show
        Lionel Herbet added a comment - The new version with Apache header included, still in git diff.
        Hide
        Johny Rufus added a comment -

        +1, Committing the latest patch.

        Show
        Johny Rufus added a comment - +1, Committing the latest patch.
        Hide
        ASF subversion and git services added a comment -

        Commit 4d7124700f678169756df81aa110bd979a4bcb93 in flume's branch refs/heads/trunk from Johny Rufus
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=4d71247 ]

        FLUME-2628. Add an optional parameter to specify the expected input text encoding for the netcat source

        (Lionel Herbet via Johny Rufus)

        Show
        ASF subversion and git services added a comment - Commit 4d7124700f678169756df81aa110bd979a4bcb93 in flume's branch refs/heads/trunk from Johny Rufus [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=4d71247 ] FLUME-2628 . Add an optional parameter to specify the expected input text encoding for the netcat source (Lionel Herbet via Johny Rufus)
        Hide
        ASF subversion and git services added a comment -

        Commit d1f3aed69b23ba46c40e6531e6ab8c3cdd181c9e in flume's branch refs/heads/flume-1.7 from Johny Rufus
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=d1f3aed ]

        FLUME-2628. Add an optional parameter to specify the expected input text encoding for the netcat source

        (Lionel Herbet via Johny Rufus)

        Show
        ASF subversion and git services added a comment - Commit d1f3aed69b23ba46c40e6531e6ab8c3cdd181c9e in flume's branch refs/heads/flume-1.7 from Johny Rufus [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=d1f3aed ] FLUME-2628 . Add an optional parameter to specify the expected input text encoding for the netcat source (Lionel Herbet via Johny Rufus)
        Hide
        Johny Rufus added a comment -

        Thanks Lionel Herbet for the patch !!

        Show
        Johny Rufus added a comment - Thanks Lionel Herbet for the patch !!
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Flume-trunk-hbase-1 #112 (See https://builds.apache.org/job/Flume-trunk-hbase-1/112/)
        FLUME-2628. Add an optional parameter to specify the expected input text encoding for the netcat source (johnyrufus: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=4d7124700f678169756df81aa110bd979a4bcb93)

        • flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java
        • flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatSource.java
        • flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java
        Show
        Hudson added a comment - FAILURE: Integrated in Flume-trunk-hbase-1 #112 (See https://builds.apache.org/job/Flume-trunk-hbase-1/112/ ) FLUME-2628 . Add an optional parameter to specify the expected input text encoding for the netcat source (johnyrufus: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=4d7124700f678169756df81aa110bd979a4bcb93 ) flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatSource.java flume-ng-core/src/main/java/org/apache/flume/source/NetcatSourceConfigurationConstants.java

          People

          • Assignee:
            Lionel Herbet
            Reporter:
            Lionel Herbet
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development