HDFS-16628 RBF: Correct target directory when move to trash for kerberos login user.#4424
Conversation
|
🎊 +1 overall
This message was automatically generated. |
Hexiaoqiao
left a comment
There was a problem hiding this comment.
@zhangxiping1 Thanks for your report and contribution. Good catch here.
Leave some comments inline. FYI.
| } | ||
|
|
||
| @Test | ||
| public void testMoveToTrashNoMountPointWithKerBoersUser() throws IOException, |
There was a problem hiding this comment.
testMoveToTrashNoMountPointWithKerBoersUser -> testMoveToTrasWithKerberosUser?
a. IMO, this case is not related with NoMountPoint or not, right?
b. KerBoers -> Kerberos.
There was a problem hiding this comment.
OK, thank you for your correction.
| public void testMoveToTrashNoMountPointWithKerBoersUser() throws IOException, | ||
| URISyntaxException, InterruptedException { | ||
| //Constructs the structure of the KerBoers user name | ||
| String kerBoersUser = "randomUser/dev@HADOOP.COM"; |
There was a problem hiding this comment.
Suggest to correct KerBoers to Kerberos. Both of the following spell issues.
| DFSClient currentUserClientNs1 = nn1Context.getClient(); | ||
|
|
||
| currentUserClientNs0.setOwner("/", ugi.getShortUserName(), ugi.getShortUserName()); | ||
| currentUserClientNs1.setOwner("/", ugi.getShortUserName(), ugi.getShortUserName()); |
There was a problem hiding this comment.
If only one DFSClient and NamenodeContext is enough for this unit test?
There was a problem hiding this comment.
It is a good idea to set the test user permissions for both subsets. Give test users permission to create both subCluster Trash directories, otherwise there may be some false positives.
| Configuration routerConf = routerContext.getConf(); | ||
| FileSystem fs = DFSTestUtil.getFileSystemAs(ugi, routerConf); | ||
| Trash trash = new Trash(fs, routerConf); | ||
| assertTrue(trash.moveToTrash(filePath)); |
There was a problem hiding this comment.
Suggest to check if the final trash directory entries are expected: /user/randomUser/.Trash/?
|
🎊 +1 overall
This message was automatically generated. |
|
cc @Hexiaoqiao It has been modified. Please help to merge it. Thank you very much |
Hexiaoqiao
left a comment
There was a problem hiding this comment.
LGTM. +1. Will commit if no more other comments for a while.
|
Ok, thank you @Hexiaoqiao |
|
Committed to trunk. Thanks @zhangxiping1 for your report and contribution. |
…ros login user. (apache#4424). Contributed by Xiping Zhang. (cherry picked from commit f8c7e67)
…ros login user. (apache#4424). Contributed by Xiping Zhang. (cherry picked from commit f8c7e67)
…ros login user. (apache#4424). Contributed by Xiping Zhang.
HDFS-16628 RBF: kerbose user remove Non-default namespace data failed