Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-767

to remove mapreduce dependency on commons-cli2

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.20.1
    • Fix Version/s: 0.20.1
    • Component/s: contrib/streaming
    • Labels:
      None
    • Release Note:
      Removes the dependency of hadoop-mapred from commons-cli2 and uses commons-cli1.2 for command-line parsing.

      Description

      mapreduce, streaming and eclipse plugin depends on common-cli2

      1. MAPREDUCE-767-v1.1.patch
        27 kB
        Amar Kamat
      2. MAPREDUCE-767-v1.2.patch
        29 kB
        Amar Kamat
      3. MAPREDUCE-767-v1.3.patch
        29 kB
        Amar Kamat
      4. MAPREDUCE-767-v1.3-branch-0.20.patch
        28 kB
        Amar Kamat

        Issue Links

          Activity

          Hide
          Amar Kamat added a comment -

          HADOOP-6213 should fix this.

          Show
          Amar Kamat added a comment - HADOOP-6213 should fix this.
          Hide
          Lee Tucker added a comment -

          It looks like the commons-cli-2.0-SNAPSHOT.jar wasn't deleted. Please fix the commit.

          Show
          Lee Tucker added a comment - It looks like the commons-cli-2.0-SNAPSHOT.jar wasn't deleted. Please fix the commit.
          Hide
          gary murry added a comment -

          Hi guys, Can we get a comment on why no new unit tests were needed? Thanks.

          Show
          gary murry added a comment - Hi guys, Can we get a comment on why no new unit tests were needed? Thanks.
          Hide
          Devaraj Das added a comment -

          I committed to trunk a fix to handle -debug (that was missed in the earlier patch).
          I committed the patch for 0.20 to the 0.20 branch. Thanks, Amar!

          Show
          Devaraj Das added a comment - I committed to trunk a fix to handle -debug (that was missed in the earlier patch). I committed the patch for 0.20 to the 0.20 branch. Thanks, Amar!
          Hide
          Amar Kamat added a comment -

          ant tests for branch-0.20 passed.

          Show
          Amar Kamat added a comment - ant tests for branch-0.20 passed.
          Hide
          Amar Kamat added a comment -

          The new patch on jira is a complete patch with the bug fixes to do with

          • split("=", 2)
          • -debug (which was missed)
          Show
          Amar Kamat added a comment - The new patch on jira is a complete patch with the bug fixes to do with split("=", 2) -debug (which was missed)
          Hide
          Amar Kamat added a comment -

          The patch for 20 is for review. Running tests for branch 20 patch.

          Show
          Amar Kamat added a comment - The patch for 20 is for review. Running tests for branch 20 patch.
          Hide
          Amar Kamat added a comment -

          Attaching a patch for 0.20. Attaching a patch for trunk with minor fix.

          Show
          Amar Kamat added a comment - Attaching a patch for 0.20. Attaching a patch for trunk with minor fix.
          Hide
          Owen O'Malley added a comment -

          +1

          I've committed this to trunk with the slight change that string.split("=",2) was missing the "2".

          We need this patch for 0.20.1. Can you make one please?

          Show
          Owen O'Malley added a comment - +1 I've committed this to trunk with the slight change that string.split("=",2) was missing the "2". We need this patch for 0.20.1. Can you make one please?
          Hide
          Amar Kamat added a comment -

          From the documentation of cli2 and cli1.2, its clear that cli1.2 doesnt support Validators and PropertyOption. But from my manual testing it seems like we can have a workaround for that. Options like -jobconf and -cmdenv extend PropertyOption. Also -file option uses Validators which can be checked once the option is parsed. I didnt see any difference in streaming command-line parsing with the patch.

          Show
          Amar Kamat added a comment - From the documentation of cli2 and cli1.2 , its clear that cli1.2 doesnt support Validators and PropertyOption . But from my manual testing it seems like we can have a workaround for that. Options like -jobconf and -cmdenv extend PropertyOption. Also -file option uses Validators which can be checked once the option is parsed. I didnt see any difference in streaming command-line parsing with the patch.
          Hide
          Amar Kamat added a comment -

          ant test (core and contrib) passed (except TestJobHistory, which is a known issue).

          Show
          Amar Kamat added a comment - ant test (core and contrib) passed (except TestJobHistory, which is a known issue).
          Hide
          Amar Kamat added a comment -

          Attaching a patch that

          • adds file check instead of validators
          • fixes some pipes bug

          Result of test-patch
          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] -1 tests included. The patch doesn't appear to include any new or modified tests.
          [exec] Please justify why no new tests are needed for this patch.
          [exec] Also please list what manual steps were performed to verify this patch.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

          ant tests (core+contrib) passed except TestJobHistory which is a known issue.

          Show
          Amar Kamat added a comment - Attaching a patch that adds file check instead of validators fixes some pipes bug Result of test-patch [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. ant tests (core+contrib) passed except TestJobHistory which is a known issue.
          Hide
          Amareshwari Sriramadasu added a comment -

          Some comments:
          1. Validators should not be removed. move them into a method and call them from streaming code.
          2. Can you check passing -jobconf x=y x1=y1 throws an exception? Can you also verify if it is easy to split the values from streaming and add them?
          3. Pipes options should also be tested

          Show
          Amareshwari Sriramadasu added a comment - Some comments: 1. Validators should not be removed. move them into a method and call them from streaming code. 2. Can you check passing -jobconf x=y x1=y1 throws an exception? Can you also verify if it is easy to split the values from streaming and add them? 3. Pipes options should also be tested
          Hide
          Amar Kamat added a comment -

          Tested this patch with examples mentioned in streaming docs. All cases seem to pass. Doing further testing.

          Show
          Amar Kamat added a comment - Tested this patch with examples mentioned in streaming docs . All cases seem to pass. Doing further testing.
          Hide
          Amar Kamat added a comment -

          All contrib tests passed.

          Show
          Amar Kamat added a comment - All contrib tests passed.
          Hide
          Amar Kamat added a comment -

          Attaching a patch that removes the dependency of mapred on cli2. Result of test-patch
          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] -1 tests included. The patch doesn't appear to include any new or modified tests.
          [exec] Please justify why no new tests are needed for this patch.
          [exec] Also please list what manual steps were performed to verify this patch.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

          ant tests passed on my box. Running tests on contrib (streaming passed). I have tried porting the cli options to cli 1.2. Testing/Reviewing the patch.

          Show
          Amar Kamat added a comment - Attaching a patch that removes the dependency of mapred on cli2. Result of test-patch [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. ant tests passed on my box. Running tests on contrib (streaming passed). I have tried porting the cli options to cli 1.2. Testing/Reviewing the patch.

            People

            • Assignee:
              Amar Kamat
              Reporter:
              Giridharan Kesavan
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development