yuqi1129 commented on code in PR #5020:
URL: https://github.com/apache/gravitino/pull/5020#discussion_r1796902289


##########
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java:
##########
@@ -742,4 +785,44 @@ private boolean checkSingleFile(Fileset fileset) {
           fileset.name());
     }
   }
+
+  static FileSystem getFileSystem(Path path, Map<String, String> config) 
throws IOException {
+    Map<String, String> newConfig = Maps.newHashMap(config);
+    String scheme;
+    Path fsPath;
+    if (path != null) {
+      scheme = path.toUri().getScheme();
+      if (scheme == null) {
+        scheme = LOCAL_FILE_SCHEMA;

Review Comment:
   > I think if the sheme is null, then it should be the one got from 
DEFAULT_FS, not the local file system.
   
   Yeah.  this is my fault. 
   
   Since the interface is `FileSystem getFileSystem(Map<String, String> 
config)`,  it requires that configuration `fs.defaultFS` is explicitly set if 
we do not provide the path when getting a filesystem instance. 
   
   > Besides, your assumption is that user have to set fs.defaultFS is config, 
right?
   
   There are the following two situations, It depends on which kind of `user` 
you refer
   
   - For ordinary users, they do not need to specify the `fs.defaultFS` when 
creating a Hadoop catalog, the value of `fs.defaultFS` will be extracted from 
the storage path they set.
   
   - For developers, due to the interface limitation, we must set 
`fs.defaultFS` explicitly, or we can't get the proper file system instance. 



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

Reply via email to