hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-5624) Add tests for ACLs in combination with viewfs.
Date Sat, 12 Jul 2014 20:22:04 GMT

    [ https://issues.apache.org/jira/browse/HDFS-5624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059910#comment-14059910

Chris Nauroth commented on HDFS-5624:

Hi, [~schu].  I've been trying to get to this one for a long time, and I really appreciate
that you picked it up.  :-)

This looks correct overall to me.  Here are some comments:

# This is implemented slightly differently from the pattern established for other mutation
operations like {{setPermission}}.  Instead of explicitly checking {{isInternalDir}}, the
established pattern is to override the method inside the {{ViewFileSystem.InternalDirOfViewFs}}
class and make the overridden method throw the exception.  Could you please change the patch
to follow that pattern?  I expect you won't need to change the methods of {{ViewFileSystem}}
at all.  You'll just need to add the new method overrides in {{ViewFileSystem.InternalDirOfViewFs}}.
# Similarly, let's make {{getAclStatus}} an override in {{ViewFileSystem.InternalDirOfViewFs}}.
 The methods in the top-level {{ViewFileSystem}} class will just be simple passthroughs after
resolving the mount.
# I see that we hadn't implemented the ACL methods in {{ViewFs}}.  It would be reasonable
to include that implementation in this patch if you want to do it, or feel free to file a
separate issue if you prefer to manage it as separate patches.  I expect the code will be
very similar to the corresponding methods in {{ViewFileSystem}}.
# I think separate test suites for ACLs and xattrs makes the most sense.  I like the flexibility
of being able to run the subset of tests relevant to a particular feature when I'm working
on its code.  However, I'd recommend naming this class {{TestViewFileSystemWithAcls}} to make
it clear that this test covers {{ViewFileSystem}}, not {{ViewFs}}.  (I would expect another
test suite to be added to go with the {{ViewFs}} implementation I mentioned above.)

I'll take another look after these comments are addressed.  Thanks again!

> Add tests for ACLs in combination with viewfs.
> ----------------------------------------------
>                 Key: HDFS-5624
>                 URL: https://issues.apache.org/jira/browse/HDFS-5624
>             Project: Hadoop HDFS
>          Issue Type: Test
>          Components: hdfs-client
>    Affects Versions: 2.4.0
>            Reporter: Chris Nauroth
>            Assignee: Stephen Chu
>         Attachments: HDFS-5624.001.patch
> Add tests verifying that in a federated deployment, a viewfs wrapped over multiple federated
NameNodes will dispatch the ACL operations to the correct NameNode.

This message was sent by Atlassian JIRA

View raw message