taklwu commented on a change in pull request #88:
URL:
https://github.com/apache/hbase-operator-tools/pull/88#discussion_r640115793
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws
IOException {
return;
}
- FileSystem fs = FileSystem.get(getConf());
+ FileSystem fs = FSUtils.getCurrentFileSystem(getConf());
Review comment:
good to know about use `CommonFSUtils` and further `HBCKFSUtils`.
for whatever I touch here, I can of coz use/implement `HBCKFSUtils`, but
again.....I felt like we should not re-implement the same thing across
`hbase-common` and `hbase-operator-tools` (sorry I'm a bit confused even I
understand about the compatibility across two repositories/projects) So, for
this change, should we use Utilities class of `CommonFSUtils` and further make
it `InterfaceAudience.Public` (sorry that I just don't like copying code)?
I'm learning, IMO the standard of using `InterfaceAudience.Private` outside
of hbase core is a bit higher that if we think we should not use any
`InterfaceAudience.Private` classes in `hbase-common` or `hbase-common` itself
should not be used in `hbase-operator-tools`, we may need to have a larger
refactoring to copy/implement few more functions from `CommonFSUtils` and
`FSUtils` to `HBCKFSUtils`. however, to limit the change/scope of this bug fix
and I found more classes in this package make use of `CommonFSUtils` and
`FSUtils`, I would propose and suggest to have a follow-up PR if needed.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
