Uploaded image for project: '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
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.5.1
    • Fix Version/s: 1.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

        Issue Links

          Activity

          Hide
          lionel_h Lionel Herbet added a comment -

          Patch fot the netcat source

          Show
          lionel_h Lionel Herbet added a comment - Patch fot the netcat source
          Hide
          jrufus 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
          jrufus 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
          jrufus 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
          jrufus 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_h 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_h 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
          jrufus 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
          jrufus 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_h 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_h 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
          jrufus Johny Rufus added a comment -

          thanks Lionel Herbet,
          +1 , the patch looks good

          Show
          jrufus Johny Rufus added a comment - thanks Lionel Herbet , +1 , the patch looks good
          Hide
          jrufus 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
          jrufus 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_h Lionel Herbet added a comment -

          Johny Rufus, the patch with the correct naming convention.

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

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

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

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

          Show
          lionel_h Lionel Herbet added a comment - Same Patch but with Git diff. I hope it will work.
          Hide
          jrufus 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
          jrufus 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_h Lionel Herbet added a comment -

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

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

          +1, Committing the latest patch.

          Show
          jrufus Johny Rufus added a comment - +1, Committing the latest patch.
          Hide
          jira-bot 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
          jira-bot 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
          jira-bot 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
          jira-bot 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
          jrufus Johny Rufus added a comment -

          Thanks Lionel Herbet for the patch !!

          Show
          jrufus Johny Rufus added a comment - Thanks Lionel Herbet for the patch !!
          Hide
          hudson 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 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_h Lionel Herbet
              Reporter:
              lionel_h Lionel Herbet
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development