You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org> on 2009/01/15 03:31:59 UTC
[jira] Created: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Remove deprecated FileSystem.isDirectory()
------------------------------------------
Key: HADOOP-5045
URL: https://issues.apache.org/jira/browse/HADOOP-5045
Project: Hadoop Core
Issue Type: Sub-task
Components: fs
Reporter: Tsz Wo (Nicholas), SZE
We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666360#action_12666360 ]
Doug Cutting commented on HADOOP-5045:
--------------------------------------
There are some whitespace changes and additions of serialVersionUID in your patch that should not be there.
The most common case we'd like to fix is where listStatus() is used, then one needlessly calls isDirectory() on each path listed rather than using the status, so the javadoc should mention listStatus() as well as getStatus(). The methods this patch modifies in FileUtil are examples of this. If the src parameter of copy() and copyMerge() were FileStatus rather than Path then a call to getStatus could be avoided. A fix is to add private methods with these new parameters, and have the public methods call getStatus and pass the value to the private methods which then walk the directory tree.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Attachments: 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12664694#action_12664694 ]
Suresh Srinivas commented on HADOOP-5045:
-----------------------------------------
I am not sure why we deprecated {{FileSystem.isDirectory(path)}} method in favor of using {{FileSystem.getFileStatus(path).isDir()}}. The method is implemented as in org.apache.hadoop.fs.FileSystem.java as:
{noformat}
/** True iff the named path is a directory. */
/** @deprecated Use getFileStatus() instead */ @Deprecated
public boolean isDirectory(Path f) throws IOException {
try {
return getFileStatus(f).isDir();
} catch (FileNotFoundException e) {
return false; // f does not exist
}
}
{noformat}
It is convenient in the following cases (there are many instance of this in the code):
{noformat}
if (fs.isDirectory(path)) {
// do something
}
{noformat}
Changing it to use {{getFileStatus}} requires catching the exception thrown by the method when the file does not exist as follows:
{noformat}
boolean directory = false;
try {
FileStatus filestatus = fs.getFileStatus(path);
directory = filestatus.isDir();
} catch (FileNotFoundException ex) {
// path does not exist
}
if (directory) {
// do something
}
{noformat}
The above code needs to be added at all the places where isDirectory() is being checked. This functionality is what the deprecated method provides, and it gives the convenience of not repeating the try-catch every where.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) FileSystem.isDirectory() should not
be deprecated.
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tsz Wo (Nicholas), SZE updated HADOOP-5045:
-------------------------------------------
Summary: FileSystem.isDirectory() should not be deprecated. (was: Remove deprecated FileSystem.isDirectory())
Edited summary: "Remove deprecated FileSystem.isDirectory()" => "FileSystem.isDirectory() should not be deprecated."
> FileSystem.isDirectory() should not be deprecated.
> --------------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Fix For: 0.20.0
>
> Attachments: 5045.new.patch, 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Homan reassigned HADOOP-5045:
-----------------------------------
Assignee: Suresh Srinivas (was: Jakob Homan)
Re-assigning to Suresh, who is farther along than I.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5045) FileSystem.isDirectory() should not
be deprecated.
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673993#action_12673993 ]
Hudson commented on HADOOP-5045:
--------------------------------
Integrated in Hadoop-trunk #756 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/756/])
> FileSystem.isDirectory() should not be deprecated.
> --------------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Fix For: 0.21.0
>
> Attachments: 5045.new.patch, 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Suresh Srinivas updated HADOOP-5045:
------------------------------------
Fix Version/s: 0.20.0
Status: Patch Available (was: Open)
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Fix For: 0.20.0
>
> Attachments: 5045.new.patch, 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tsz Wo (Nicholas), SZE updated HADOOP-5045:
-------------------------------------------
Hadoop Flags: [Reviewed] (was: [Incompatible change])
+1 patch looks good except there are white space changes.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Attachments: 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Suresh Srinivas updated HADOOP-5045:
------------------------------------
Attachment: 5045.patch
I have attached new patch to incorporate Doug's comments. I have made use of {{FileStatus}} returned by {{listStatus()}} and removed white space changes.
My comments on suggestions from Nicholas:
* We should use fs.getFileStatus(src).isDir(), instead of fs.isDirectory(src) , in the cases that src must exist or the operation must abort. Found these cases in
o ChecksumFileSystem line 402 and line 499
o FsShell line 1034 and line 1087
> This cannot be done without further changes. Today all these method return false when a file does not exist. The methods will throw FileNotFoundException instead of returning false with this change.
* FsShell.copyFromStdin(..) should use getFileStatus for checking exist and isDir in one shot.
> Again in this case the exception thrown would be different which results in unit testcase failure.
* In FsShell line 835, srcFs.isDirectory(dst) looks like a bug since it uses srcFs with dst path.
> Looks like becase the operation is rename the expectation is that both the source and the destination file systems are of the same type. The code could have been better orgainized if we had only one filesystem variable in the method instead of two. I have made that change.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Attachments: 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Homan reassigned HADOOP-5045:
-----------------------------------
Assignee: Jakob Homan
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Jakob Homan
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Suresh Srinivas updated HADOOP-5045:
------------------------------------
Attachment: 5045.new.patch
New patch with white space changes removed. Thanks Nicholas for catching it.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Fix For: 0.20.0
>
> Attachments: 5045.new.patch, 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) FileSystem.isDirectory() should not
be deprecated.
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tsz Wo (Nicholas), SZE updated HADOOP-5045:
-------------------------------------------
Resolution: Fixed
Fix Version/s: (was: 0.20.0)
0.21.0
Status: Resolved (was: Patch Available)
I just committed this. Thanks, Suresh!
> FileSystem.isDirectory() should not be deprecated.
> --------------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Fix For: 0.21.0
>
> Attachments: 5045.new.patch, 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668465#action_12668465 ]
Hadoop QA commented on HADOOP-5045:
-----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12398946/5045.new.patch
against trunk revision 738744.
+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 warnings.
+1 Eclipse classpath. The patch retains Eclipse classpath integrity.
+1 core tests. The patch passed core unit tests.
-1 contrib tests. The patch failed contrib unit tests.
Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3772/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3772/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3772/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3772/console
This message is automatically generated.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Fix For: 0.20.0
>
> Attachments: 5045.new.patch, 5045.patch, 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665665#action_12665665 ]
Doug Cutting commented on HADOOP-5045:
--------------------------------------
The motivation for deprecating such Path-based methods was that, in many cases other file operations were made concerning the same path, and namenode load could be minimized if listStatus() or getStatus() were used. So we should not blindly replace all calls with some boilerplate, but rather examine each. For example, if the directory is itself from a directory listing, then a FileStatus could already be available. If we find that there are many calls to isDirectory() that really stand alone, where a path is directly provided by user code, then we could consider de-deprecating this method, and instead adding a comment cautioning developers to try to use listStatus() or getStatus() instead to minimize namenode load.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666362#action_12666362 ]
Tsz Wo (Nicholas), SZE commented on HADOOP-5045:
------------------------------------------------
- We should use fs.getFileStatus(src).isDir(), instead of fs.isDirectory(src) , in the cases that src must exist or the operation must abort. Found these cases in
-* ChecksumFileSystem line 402 and line 499
-* FsShell line 1034 and line 1087
- FsShell.copyFromStdin(..) should use getFileStatus for checking exist and isDir in one shot.
- In FsShell line 835, srcFs.isDirectory(dst) looks like a bug since it uses srcFs with dst path.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Attachments: 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5045) Remove deprecated
FileSystem.isDirectory()
Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Suresh Srinivas updated HADOOP-5045:
------------------------------------
Attachment: 5045.patch
FileSystem currently has two convenient methods - {{isDirectory(path)}} and {{isFile(path)}}. Both of this can cause some one to write code that creates {{FileStatus}} unnecessarily. However previously only {{isDirectory(path)}} was deprecated.
There are many instances in the code that are using both these methods. I think it is a convenient methods that prevents having multiple instances of equivalent implementation. My suggestion is to de-deprecate the method. I have attached a patch to do the same. The patch also modifies code to reuse {{FileStatus}} where it is possible.
> Remove deprecated FileSystem.isDirectory()
> ------------------------------------------
>
> Key: HADOOP-5045
> URL: https://issues.apache.org/jira/browse/HADOOP-5045
> Project: Hadoop Core
> Issue Type: Sub-task
> Components: fs
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Suresh Srinivas
> Attachments: 5045.patch
>
>
> We should remove FileSystem.isDirectory().
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.