Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The FsShell has many chains if/then/else chains for instantiating and running commands. A dynamic mechanism is needed for registering commands such that FsShell requires no changes when adding new commands.

      1. HADOOP-7224-2.patch
        18 kB
        Daryn Sharp
      2. HADOOP-7224.patch
        17 kB
        Daryn Sharp

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #661 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/661/)
          Add the missing file TestCommandFactory for HADOOP-7224.
          HADOOP-7224. Add CommandFactory to shell. Contributed by Daryn Sharp

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #661 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/661/ ) Add the missing file TestCommandFactory for HADOOP-7224 . HADOOP-7224 . Add CommandFactory to shell. Contributed by Daryn Sharp
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #552 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/552/)
          Add the missing file TestCommandFactory for HADOOP-7224.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #552 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/552/ ) Add the missing file TestCommandFactory for HADOOP-7224 .
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #550 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/550/)
          HADOOP-7224. Add CommandFactory to shell. Contributed by Daryn Sharp

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #550 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/550/ ) HADOOP-7224 . Add CommandFactory to shell. Contributed by Daryn Sharp
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks Daryn!

          Also thanks Tanping for reviewing it.

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks Daryn! Also thanks Tanping for reviewing it.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12476268/HADOOP-7224-2.patch
          against trunk revision 1091618.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/347//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/347//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/347//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12476268/HADOOP-7224-2.patch against trunk revision 1091618. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/347//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/347//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/347//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          (I'm just updating comments in the patch)

          The factory has a registerCommands(Class) method in order to abstract the factory's contract with its commands. Doing so will allow the api to evolve w/o impacting the caller (such as FsShell).

          I eventually plan to add a method to FsShell to return the registrar class so subclasses can return their own class for commands. These subclasses will be insulated from the details of how and when the factory is initialized.

          Show
          Daryn Sharp added a comment - (I'm just updating comments in the patch) The factory has a registerCommands(Class) method in order to abstract the factory's contract with its commands. Doing so will allow the api to evolve w/o impacting the caller (such as FsShell). I eventually plan to add a method to FsShell to return the registrar class so subclasses can return their own class for commands. These subclasses will be insulated from the details of how and when the factory is initialized.
          Hide
          Tanping Wang added a comment -

          +1. Dyran explained to me how

            public void registerCommands(Class<?> registrarClass)
          

          in CommandFactory.java is planned to be used and the reason why it is needed. I think it is a good idea. Please clarify here on the Jira.

          Show
          Tanping Wang added a comment - +1. Dyran explained to me how public void registerCommands( Class <?> registrarClass) in CommandFactory.java is planned to be used and the reason why it is needed. I think it is a good idea. Please clarify here on the Jira.
          Hide
          Daryn Sharp added a comment -

          [exec] +1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 3 new or modified tests.
          [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 (version 1.3.9) warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system test framework. The patch passed system test framework compile.

          Show
          Daryn Sharp added a comment - [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [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 (version 1.3.9) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system test framework. The patch passed system test framework compile.

            People

            • Assignee:
              Daryn Sharp
              Reporter:
              Daryn Sharp
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development