[
https://issues.apache.org/jira/browse/HADOOP-18122?focusedWorklogId=725353&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-725353
]
ASF GitHub Bot logged work on HADOOP-18122:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 11/Feb/22 18:49
Start Date: 11/Feb/22 18:49
Worklog Time Spent: 10m
Work Description: ayushtkn commented on a change in pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#discussion_r804901362
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1778,15 +1793,34 @@ public BlockStoragePolicySpi getStoragePolicy(Path src)
throws IOException {
Collection<BlockStoragePolicySpi> allPolicies = new HashSet<>();
for (FileSystem fs : getChildFileSystems()) {
try {
- Collection<? extends BlockStoragePolicySpi> policies =
- fs.getAllStoragePolicies();
+ Collection<? extends BlockStoragePolicySpi> policies =
fs.getAllStoragePolicies();
Review comment:
unrelated change
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -1479,16 +1484,12 @@ public void
testTargetFileSystemLazyInitializationForChecksumMethods()
final String clusterName = "cluster" + new Random().nextInt();
Configuration config = new Configuration(conf);
config.setBoolean(CONFIG_VIEWFS_ENABLE_INNER_CACHE, false);
- config.setClass("fs.othermockfs.impl",
- TestChRootedFileSystem.MockFileSystem.class, FileSystem.class);
- ConfigUtil.addLink(config, clusterName, "/user",
- URI.create("othermockfs://mockauth1/mockpath"));
- ConfigUtil.addLink(config, clusterName,
- "/mock", URI.create("othermockfs://mockauth/mockpath"));
+ config.setClass("fs.othermockfs.impl",
TestChRootedFileSystem.MockFileSystem.class, FileSystem.class);
+ ConfigUtil.addLink(config, clusterName, "/user",
URI.create("othermockfs://mockauth1/mockpath"));
+ ConfigUtil.addLink(config, clusterName, "/mock",
URI.create("othermockfs://mockauth/mockpath"));
Review comment:
looks like just formatting change, can you please remove the just
formatting changes, here and other places as well.
We should restrict ourselves to only related changes,
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1778,15 +1793,34 @@ public BlockStoragePolicySpi getStoragePolicy(Path src)
throws IOException {
Collection<BlockStoragePolicySpi> allPolicies = new HashSet<>();
for (FileSystem fs : getChildFileSystems()) {
try {
- Collection<? extends BlockStoragePolicySpi> policies =
- fs.getAllStoragePolicies();
+ Collection<? extends BlockStoragePolicySpi> policies =
fs.getAllStoragePolicies();
allPolicies.addAll(policies);
} catch (UnsupportedOperationException e) {
// ignored
}
}
return allPolicies;
}
+
+ private FsPermission getMountLinkDefaultPermissions() {
+ return PERMISSION_555;
+ }
+
+ private String getMountLinkUserName() {
+ String username = config.get(CONFIG_VIEWFS_MOUNT_LINKS_USER_NAME);
Review comment:
we would be fetching the value everytime from the config for every
operation? why not get once and then store it. the config object won't change
post FS has been initialised?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1682,9 +1696,10 @@ public void setAcl(Path path, List<AclEntry> aclSpec)
throws IOException {
@Override
public AclStatus getAclStatus(Path path) throws IOException {
checkPathIsSlash(path);
- return new AclStatus.Builder().owner(ugi.getShortUserName())
- .group(ugi.getPrimaryGroupName())
- .addEntries(AclUtil.getMinimalAcl(PERMISSION_555))
+ return new AclStatus.Builder().owner(getMountLinkUserName())
+ .group(getMountLinkGroupName())
+ .setPermission(PERMISSION_555)
Review comment:
why not getMountLinkDefaultPermissions
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -39,6 +39,11 @@
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.FileSystemTestHelper;
+
+import static
org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_USER_NAME;
+import static
org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_GROUP_NAME;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.spy;
Review comment:
import order is wrong, should be with other static imports
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 725353)
Time Spent: 20m (was: 10m)
> ViewFileSystem fails on determining owning group when primary group doesn't
> exist for user
> ------------------------------------------------------------------------------------------
>
> Key: HADOOP-18122
> URL: https://issues.apache.org/jira/browse/HADOOP-18122
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Chentao Yu
> Assignee: Chentao Yu
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> ViewFileSystem should not fail on determining owning group when primary group
> doesn't exist for user
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
