Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1736

Break dependency between DatanodeJspHelper and FsShell

    Details

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

      Description

      DatanodeJspHelper has an artificial dependency on a date formatter field in FsShell. A pending bug is reorganizing the FsShell commands so this field will no longer be available. The dependency should be broken by having DataNodeJspHelper contain its own independent date formatter.

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #643 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/643/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #643 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/643/ )
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Daryn!

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Daryn!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #555 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/555/)
          HDFS-1736. Remove the dependency from DatanodeJspHelper to FsShell. Contributed by Daryn Sharp

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #555 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/555/ ) HDFS-1736 . Remove the dependency from DatanodeJspHelper to FsShell. Contributed by Daryn Sharp
          Hide
          Daryn Sharp added a comment -

          Of note, the failed tests are unrelated to this change.

          Test Result (3 failures / -1)
          org.apache.hadoop.hdfs.server.balancer.TestBalancer.testBalancer0
          org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermit
          org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermitQualified

          Show
          Daryn Sharp added a comment - Of note, the failed tests are unrelated to this change. Test Result (3 failures / -1) org.apache.hadoop.hdfs.server.balancer.TestBalancer.testBalancer0 org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermit org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermitQualified
          Hide
          Daryn Sharp added a comment -

          Many of the FsShell commands have systemic issues with returning non-zero on exit, especially when handling multiple arguments. I have linked example bugs, some of which are resolved, but didn't fully address the reported issue.

          Will finish the job of converting all FsShell commands to use fs.shell.Command base class (Count already does) which will indirectly fix all the exit code problems via a common code path in the base class that correctly handles paths, tracking errors, etc. The conversion will be moving this date formatter into the appropriate command's subclass.

          Show
          Daryn Sharp added a comment - Many of the FsShell commands have systemic issues with returning non-zero on exit, especially when handling multiple arguments. I have linked example bugs, some of which are resolved, but didn't fully address the reported issue. Will finish the job of converting all FsShell commands to use fs.shell.Command base class (Count already does) which will indirectly fix all the exit code problems via a common code path in the base class that correctly handles paths, tracking errors, etc. The conversion will be moving this date formatter into the appropriate command's subclass.
          Hide
          Jakob Homan added a comment -

          Would you mind linking this JIRA to them?

          Show
          Jakob Homan added a comment - Would you mind linking this JIRA to them?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12473065/HDFS-1736.patch
          against trunk revision 1079069.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 failed these core unit tests:
          org.apache.hadoop.hdfs.server.balancer.TestBalancer

          -1 contrib tests. The patch failed contrib unit tests.

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

          Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/241//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/241//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/241//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/12473065/HDFS-1736.patch against trunk revision 1079069. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 failed these core unit tests: org.apache.hadoop.hdfs.server.balancer.TestBalancer -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/241//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/241//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/241//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          There are many open bugs regarding incorrect exit codes from various FsShell commands. I'll be resolving all of them.

          Show
          Daryn Sharp added a comment - There are many open bugs regarding incorrect exit codes from various FsShell commands. I'll be resolving all of them.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
          Hide
          Jakob Homan added a comment -

          A pending bug is reorganizing the FsShell commands so this field will no longer be available.

          Which pending bug?

          Show
          Jakob Homan added a comment - A pending bug is reorganizing the FsShell commands so this field will no longer be available. Which pending bug?
          Hide
          Daryn Sharp added a comment -

          There are no tests because the output of the page has not changed.

          Show
          Daryn Sharp added a comment - There are no tests because the output of the page has not changed.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 24h
                24h
                Remaining:
                Remaining Estimate - 24h
                24h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development