[
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
(v6.2#6252)
