Use the same shared FileSystem instance across calls in HadoopTableOperations #108
Conversation
| @@ -76,6 +76,10 @@ public static HadoopInputFile fromStatus(FileStatus stat, Configuration conf) { | |||
| } | |||
| } | |||
|
|
|||
| static HadoopInputFile fromFsPath(FileSystem fs, Path path, Configuration conf) { | |||
There was a problem hiding this comment.
Instead of adding this, I think HadoopInputFile should make its constructor protected to allow you to create a subclass.
The reason why we use Path and Configuration is to ensure that the file system always matches the path's scheme. Adding a new factory method that allows users to supply a file system creates a path around that guarantee. That's legitimate for your use case, but you'll need to implement HadoopTableOperations anyway, so I think a safer way to solve the problem for you is to subclass HadoopInputFile and expose this method in your private subclass.
There was a problem hiding this comment.
I should note that for the limited use in this PR I think it is okay because this is package-private. It would just be better to solve your overall use case with a custom TableOperations implementation.
| private TableMetadata currentMetadata = null; | ||
| private Integer version = null; | ||
| private boolean shouldRefresh = true; | ||
|
|
||
| HadoopTableOperations(Path location, Configuration conf) { | ||
| this.conf = conf; | ||
| this.location = location; | ||
| this.metadataFs = Util.getFS(location, conf); |
There was a problem hiding this comment.
These changes look fine to me, along with the package-private changes to HadoopInputFile and HadoopOutputFile.
|
This was migrated to apache/iceberg#14. |
Closes #92.