virajjasani commented on pull request #3730:
URL: https://github.com/apache/hadoop/pull/3730#issuecomment-983647264


   Thanks for taking a look.
   
   > I'm not confident that making the getSnapshotDiffReportListing of 
DistributedFileSystem and WebHdfsFileSystem public is right while I have no 
idea of alternative.
   
   I have made WebHdfsFileSystem#getSnapshotDiffReportListing public only 
because of test usage, so let me make it as `@VisibleForTesting` and provide 
comment as well.
   
   
   > Exposing DFSClient to HttpFS is not good.
   
   On the other hand, almost every other HttpFS APIs use DFSClient (through 
DFS) e.g. `getSnapshotListing`, `getSnapshottableDirListing`, even `DFS#access` 
is used directly by HttpFS. Though these APIs are accessed only if DFS is used 
(same logic applicable for this change). I am wondering what problem it can 
cause here. Is there any known issues similar to WebHdfsFileSystem using 
NameNode APIs directly (i.e. `NamenodeWebHdfsMethods` should utilize RPC Client 
Protocol rather than accessing FileSystem APIs directly).
   
   
   > HttpFSFileSystem might be able to leverage 
`DFSUtilClient#getSnapshotDiffReport`. It can be addressed in follow-up JIRAs.
   
   Sounds good, however, I have the same question as above that majority of 
HttpFS APIs still directly use DFSClient if the underlying FileSystem is DFS. 
Does this cause any known issues?
   Also, I just again took a look at majority sub-classes of `FSOperations` 
that are used by HttpFs and majority of them are directly using FileSystem APIs 
(for DFS specific functionality, we check whether underlying FS is DFS, if not 
we throw `UnsupportedOperationException`).
   
   Thanks @iwasakims, let me add `@VFT` on 
WebHdfsFileSystem#getSnapshotDiffReportListing to be more specific about it's 
usage policy.


-- 
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]

Reply via email to