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

ASF GitHub Bot commented on HDFS-16963:
---------------------------------------

saxenapranav commented on code in PR #5505:
URL: https://github.com/apache/hadoop/pull/5505#discussion_r1146051213


##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java:
##########
@@ -3876,10 +3873,8 @@ public boolean hasPathCapability(final Path path, final 
String capability)
       throws IOException {
     // qualify the path to make sure that it refers to the current FS.
     final Path p = makeQualified(path);
-    Optional<Boolean> cap = DfsPathCapabilities.hasPathCapability(p,
-        capability);
-    if (cap.isPresent()) {
-      return cap.get();
+    if (DfsPathCapabilities.hasPathCapability(p, capability)) {

Review Comment:
   Same comment as given in WebHdfsFileSystem.



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java:
##########
@@ -2207,10 +2206,8 @@ public boolean hasPathCapability(final Path path, final 
String capability)
       throws IOException {
     // qualify the path to make sure that it refers to the current FS.
     final Path p = makeQualified(path);
-    Optional<Boolean> cap = DfsPathCapabilities.hasPathCapability(p,
-        capability);
-    if (cap.isPresent()) {
-      return cap.get();
+    if (DfsPathCapabilities.hasPathCapability(p, capability)) {

Review Comment:
   what if `validatePathCapabilityArgs(path, capability)` in 
DfsPathCapablity.hasPathCapablity gives `CommonPathCapabilities.FS_SYMLINKS`. 
Now, `FileSystem.areSymlinksEnabled()` can be true or false. 
   In earlier code, if `FileSystem.areSymlinksEnabled()` is false, we would 
retrn from old-line 2213 `cap.get()`. But now, it will go ahead and invoke 
`super.hasPathCapability(p, capability);`





> Remove the unnecessary use of Optional from DistributedFileSystem
> -----------------------------------------------------------------
>
>                 Key: HDFS-16963
>                 URL: https://issues.apache.org/jira/browse/HDFS-16963
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: fs
>            Reporter: Tsz-wo Sze
>            Assignee: Tsz-wo Sze
>            Priority: Major
>              Labels: pull-request-available
>
> - In DfsPathCapabilities, the hasPathCapability(..) method may simply returns 
> boolean.
>  - In HdfsPathHandle, a constructor declares Optional parameters. It is a 
> well known misuse of Optional.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to