Hadoop Common
  1. Hadoop Common
  2. HADOOP-5853

Undeprecate HttpServer.addInternalServlet method to fix javac warnings

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Deprecated method HttpServer.addInternalServlet() causes many java warnings. Next comment covers the reason for undeprecating the method.

      1. 5853.patch
        1.0 kB
        Suresh Srinivas

        Issue Links

          Activity

          Suresh Srinivas created issue -
          Suresh Srinivas made changes -
          Field Original Value New Value
          Link This issue blocks HADOOP-3854 [ HADOOP-3854 ]
          Hide
          Suresh Srinivas added a comment -

          HttpServer supports adding two types of servlets - external and internal servlets. External servlets are user facing servlets and servlet filters can be added for the URLs corresponding the servlets. For internal servlets, filters are not added. In Hadoop-3854, not attaching filter was considered to be deficiency, and hence the corresponding method HttpServer.addInternalServlet() was deprecated to track this.

          Current thought is to undeprecate the method to fix java warnings. If the need arises enable/add functionality to enable filters for internal servllets with a separate jira.

          Show
          Suresh Srinivas added a comment - HttpServer supports adding two types of servlets - external and internal servlets. External servlets are user facing servlets and servlet filters can be added for the URLs corresponding the servlets. For internal servlets, filters are not added. In Hadoop-3854, not attaching filter was considered to be deficiency, and hence the corresponding method HttpServer.addInternalServlet() was deprecated to track this. Current thought is to undeprecate the method to fix java warnings. If the need arises enable/add functionality to enable filters for internal servllets with a separate jira.
          Suresh Srinivas made changes -
          Attachment 5853.patch [ 12408285 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good

          (should this fix javac warnings instead findbugs?)

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good (should this fix javac warnings instead findbugs?)
          Tsz Wo Nicholas Sze made changes -
          Hadoop Flags [Reviewed]
          Suresh Srinivas made changes -
          Summary Undeprecate HttpServer.addInternalServlet method to fix findbugs warnings Undeprecate HttpServer.addInternalServlet method to fix javac warnings
          Hide
          Suresh Srinivas added a comment -

          Here is the result of test-patch result. No test cases added because the change only undeprecates a method. I am not sure what is triggering eclipse classpath and release audit warnings
          [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 tests are needed for 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 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories.
          [exec]
          [exec] -1 release audit. The applied patch generated 490 release audit warnings (more than the trunk's current 488 warnings).

          Show
          Suresh Srinivas added a comment - Here is the result of test-patch result. No test cases added because the change only undeprecates a method. I am not sure what is triggering eclipse classpath and release audit warnings [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 tests are needed for 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 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories. [exec] [exec] -1 release audit. The applied patch generated 490 release audit warnings (more than the trunk's current 488 warnings).
          Hide
          Tsz Wo Nicholas Sze added a comment -

          > I am not sure what is triggering eclipse classpath and release audit warnings

          I think you might have done something wrong. Could you try ant test-patch again?

          Show
          Tsz Wo Nicholas Sze added a comment - > I am not sure what is triggering eclipse classpath and release audit warnings I think you might have done something wrong. Could you try ant test-patch again?
          Suresh Srinivas made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Suresh Srinivas added a comment -

          After running the tests again, I do not see eclipse classpath issue. However the release audit is still being printed. The two new warnings are:
          [javadoc] JDiff: warning: change from deprecated to undeprecated for method FileSystem.isDirectory
          [javadoc] JDiff: warning: change from deprecated to undeprecated for method HttpServer.addInternalServlet

          First one comes from a change that was made earlier. The second undeprecation is the intent of this patch. I propose committing this patch, given the release audit warning is expected.

          [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 tests are needed for 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 Eclipse classpath. The patch retains Eclipse classpath integrity.
          [exec]
          [exec] -1 release audit. The applied patch generated 496 release audit warnings (more than the trunk's current 494 warnings).

          Show
          Suresh Srinivas added a comment - After running the tests again, I do not see eclipse classpath issue. However the release audit is still being printed. The two new warnings are: [javadoc] JDiff: warning: change from deprecated to undeprecated for method FileSystem.isDirectory [javadoc] JDiff: warning: change from deprecated to undeprecated for method HttpServer.addInternalServlet First one comes from a change that was made earlier. The second undeprecation is the intent of this patch. I propose committing this patch, given the release audit warning is expected. [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 tests are needed for 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 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] -1 release audit. The applied patch generated 496 release audit warnings (more than the trunk's current 494 warnings).
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Suresh!

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Suresh!
          Tsz Wo Nicholas Sze made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          3h 27m 1 Suresh Srinivas 16/May/09 02:34
          Patch Available Patch Available Resolved Resolved
          3d 17h 23m 1 Tsz Wo Nicholas Sze 19/May/09 19:58
          Resolved Resolved Closed Closed
          462d 1h 39m 1 Tom White 24/Aug/10 21:37

            People

            • Assignee:
              Suresh Srinivas
              Reporter:
              Suresh Srinivas
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development