Trash URI semantics can be relaxed

• Key: HADOOP-5086
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Chris Douglas
• Reporter: Chris Douglas
• Component/s: fs
• Affects Version/s: 0.18.0
• Fix Version/s: 0.19.1

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.

Comments

[Chris Douglas]

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).

[Doug Cutting]

> 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.

[Chris Douglas]

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.

[Doug Cutting]

+1 this looks good to me.

[Chris Douglas]

     [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.

[Chris Douglas]

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.

[Doug Cutting]

> 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.

[Chris Douglas]

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.