Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.18.0
    • Fix Version/s: 0.19.1
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      When using fully qualified URIs with FsShell, the authority element of the URI must match the default filesystem exactly, or else one may get an error message when the trash is enabled:

      $ hadoop fs -rmr hdfs://namenode1/user/foo/bar
      rmr: Wrong FS: hdfs://namenode1/user/foo/bar, expected: hdfs://namenode1.foobar.com
      Usage: java FsShell [-rmr <path>]
      $ hadoop fs -rmr hdfs://namenode1.foobar.com/user/foo/bar
      $
      

      It should be possible to use the FileSystem for the Path provided rather than the default FileSystem. 0.17 was less particular about this.

      1. 5086-0.patch
        5 kB
        Chris Douglas
      2. 5086-1.patch
        23 kB
        Chris Douglas

        Activity

        Hide
        Chris Douglas added a comment -

        We could, in theory, merge a fix to the 0.19 [0.18?] and 0.19 branches too, if we wanted.

        Soright. I'll hold off on 0.18, since the vote for that has already been called.

        I committed this.

        Show
        Chris Douglas added a comment - We could, in theory, merge a fix to the 0.19 [0.18?] and 0.19 branches too, if we wanted. Soright. I'll hold off on 0.18, since the vote for that has already been called. I committed this.
        Hide
        Doug Cutting added a comment -

        > Should this go into 0.20? This changed between 0.17 and 0.18, but not between 0.18 and trunk.

        +1 It's still a regression, even if we were slow to notice it. We could, in theory, merge a fix to the 0.19 and 0.19 branches too, if we wanted.

        Show
        Doug Cutting added a comment - > Should this go into 0.20? This changed between 0.17 and 0.18, but not between 0.18 and trunk. +1 It's still a regression, even if we were slow to notice it. We could, in theory, merge a fix to the 0.19 and 0.19 branches too, if we wanted.
        Hide
        Chris Douglas added a comment -

        Passed all unit tests on my machine.

        Should this go into 0.20? This changed between 0.17 and 0.18, but not between 0.18 and trunk.

        Show
        Chris Douglas added a comment - Passed all unit tests on my machine. Should this go into 0.20? This changed between 0.17 and 0.18, but not between 0.18 and trunk.
        Hide
        Chris Douglas 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 15 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 warnings.
             [exec] 
             [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
        
        Show
        Chris Douglas 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 15 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 warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
        Hide
        Doug Cutting added a comment -

        +1 this looks good to me.

        Show
        Doug Cutting added a comment - +1 this looks good to me.
        Hide
        Chris Douglas added a comment -

        it would be useful if the test were reusable by HDFS, so that it could, e.g., subclass the core test to run things on a mini cluster.

        Good idea. Moved trash tests to fs, HDFS test of trash to a subclass.

        Show
        Chris Douglas added a comment - it would be useful if the test were reusable by HDFS, so that it could, e.g., subclass the core test to run things on a mini cluster. Good idea. Moved trash tests to fs, HDFS test of trash to a subclass.
        Hide
        Doug Cutting added a comment -

        > shouldn't the unit test be in o.a.h.fs?

        Yes, the generic bits certainly. And it would be useful if the test were reusable by HDFS, so that it could, e.g., subclass the core test to run things on a mini cluster.

        Show
        Doug Cutting added a comment - > shouldn't the unit test be in o.a.h.fs? Yes, the generic bits certainly. And it would be useful if the test were reusable by HDFS, so that it could, e.g., subclass the core test to run things on a mini cluster.
        Hide
        Chris Douglas added a comment -

        Attaching patch shuffling cstr params around, added a unit test (shouldn't the unit test be in o.a.h.fs? If it were moved, this could use Trash::getCurrentTrashDir instead of hard-coding its params).

        Show
        Chris Douglas added a comment - Attaching patch shuffling cstr params around, added a unit test (shouldn't the unit test be in o.a.h.fs? If it were moved, this could use Trash::getCurrentTrashDir instead of hard-coding its params).

          People

          • Assignee:
            Chris Douglas
            Reporter:
            Chris Douglas
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development