|
[
Permlink
| « Hide
]
Mahadev konar added a comment - 31/Jan/08 09:28 PM
here is a patch that fixes the issue.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12374503/patch_2194_1.patch  against trunk revision 616796. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1726/testReport/ This message is automatically generated. This patch adds an additional RPC to the namenode. I think it would be better if DFS.open can throw a FileNotFound exception when the file does not exist.
an additional RPC is added to FSShell which is not programtically used by the users. so there would not be a performance hit for the api. open throwing an exception would inlvolve parsing the remote exception string which I think is not a good idea.
Shell commands can be used in the batch mode. Some users programtically use FsShell to take advantage of the features like trash bin.
this is a patch with doing a rcp call and parses the remoteexception.
i meant without doing a rpc call
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12374811/patch_2194_2.patch against trunk revision 616796. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1744/testReport/ This message is automatically generated. This is already doing a fs.isDirectory(), so we can find if the file exists without an extra rpc call and without parsing IOException. We need to use fs.getFileStatus() instead of fs.isDirectory().
patch that reduces a few lines and keeps track of the remote exception in filenotfound exception.
patch that uses getFIleStatus and fixes filenotfoundexception thrown by getFIleStatus.
patch_2194_4.patch: +1
+1 The code looks good.
One more thought... +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12374832/patch_2194_4.patch against trunk revision 616796. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1745/testReport/ This message is automatically generated. This is a generic issue, what I mean is that there are many portions of client code that needs to map a RemoteException to a more specifiex Exception. Most client side code somehow maps a Remote Exception to a printable string that the user can understand. Maybe it makes more sense to have a new method in the RemoteException class itself that parses the remote exception string and generates an appropriate exception (and/or message) that is relevant to the Client code. This will ensure that the mapping of the RemoteException to the client exception/message remain localized to one place instead of being scattered all around in the client code.
last patch with a unit test .
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12374902/patch_2194_5.patch against trunk revision 616796. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1752/testReport/ This message is automatically generated. I just committed this. Thanks Mahadev!
Integrated in Hadoop-trunk #394 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/394/
) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
dfs cat on a file that does not exist throws a java IOException
A fundamental fix to this problem should let FileSystem.open throws a FileNotFoundException when the input path is a directory or the path does not exist. Then there is no need to check if the path is a directory before calling open.