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.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
