ayushtkn commented on a change in pull request #2061:
URL: https://github.com/apache/hadoop/pull/2061#discussion_r436837228
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1202,19 +1202,18 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
try {
String linkedPath = link.getTargetFileSystem().getUri().getPath();
- if("".equals(linkedPath)) {
+ if ("".equals(linkedPath)) {
linkedPath = "/";
}
FileStatus status =
- ((ChRootedFileSystem)link.getTargetFileSystem())
- .getMyFs().getFileStatus(new Path(linkedPath));
- result[i++] = new FileStatus(status.getLen(), false,
- status.getReplication(), status.getBlockSize(),
- status.getModificationTime(), status.getAccessTime(),
- status.getPermission(), status.getOwner(), status.getGroup(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ ((ChRootedFileSystem) link.getTargetFileSystem()).getMyFs()
+ .getFileStatus(new Path(linkedPath));
+ result[i++] = new FileStatus(status.getLen(), status.isDirectory(),
+ status.getReplication(), status.getBlockSize(),
+ status.getModificationTime(), status.getAccessTime(),
+ status.getPermission(), status.getOwner(), status.getGroup(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(myUri, null));
Review comment:
Well things are different at different places and TBH I don't have a
strong opinion on which is the best way to do.
On a personal note changing `getFileStatus()` seems to be little more safe
to me, As those assertions and stuffs stays as it is and changes gets
restricted to `viewFS` only and no changes to links interpretations and stuffs.
(my assumption, it should be safe, haven't digged in much)
```
return new FileStatus(0, true, 0, 0, creationTime, creationTime,
PERMISSION_555, ugi.getShortUserName(), ugi.getPrimaryGroupName(),
new Path(theInternalDir.fullPath).makeQualified(
myUri, ROOT_PATH));
```
`getFileStatus()` is treating it as a link only(but with isDir true), it
doesn't shows target file system permissions and times. That also need to be
changed similarly to HADOOP-17029, to resolve permissions and stuff from
target file system. FileStatus should be same through both API's? We can make
things get in sync by doing that, and get away with inconsistencies b/w these
two API's as of now..
Changes in `getListing()` apart from making the API`s in sync(That also we
need to change in getFileStatus() as well since there 'true' is hardcoded), we
seems to change symlink interpretation logics as well to get that in sync with
other systems and I think that might break things for people relying on checks
like this : `if (isDir==false and link!=null)`, May be we can have a follow up
JIRA as well, to change link interpretations with bigger audience.
But in any case, I don't have oppositions to any of the approach, all up to
you whichever way you want to go ahead. :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
