[
https://issues.apache.org/jira/browse/HDFS-16963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17704454#comment-17704454
]
ASF GitHub Bot commented on HDFS-16963:
---------------------------------------
saxenapranav commented on code in PR #5505:
URL: https://github.com/apache/hadoop/pull/5505#discussion_r1147101903
##########
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:
Hi. I understand that at present making call to
`FileSystem.hasPathCapability()` is not going to make any issue as it checks
for symlinks. But what if there is more logic added in the
`FileSystem.hasPathCapability()` in future which expects the child classes to
call the super method only in required cases.
My thought is that what ever is in `FileSystem.hasPathCapablity` should be
abstract to any dev reading `hasPathCapablity` in `DistributedFileSystem` and
`WebHdfsFileSystem`, and super.hasPathCapablity should be called only when
required. if `Optional<Boolean> cap` gets true/false, it should return from
there and not go to the super method. In my view, super method should only get
invoked in case given capability doesn't match any of the
https://github.com/apache/hadoop/blob/b9fa5e0182c19adc4ff4cd2d9265a36ce9913178/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/DfsPathCapabilities.java#L45-L57.
I feel having an enum which can say TRUE, FALSE, UNKNOWN would be great. And
the super method getting called only if `DfsPathCapablities.hasPathCapablity`
gives UNKNOWN.
Thanks.
> 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]
