jackye1995 commented on a change in pull request #1618:
URL: https://github.com/apache/iceberg/pull/1618#discussion_r505980695
##########
File path:
core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -174,7 +176,13 @@ public void commit(TableMetadata base, TableMetadata
metadata) {
@Override
public FileIO io() {
if (defaultFileIo == null) {
- defaultFileIo = new HadoopFileIO(conf);
+ defaultFileIo = ClassLoaderUtil.fromProperty(
+ current() == null ? new HashMap<>() : current().properties(),
+ TableProperties.WRITE_FILE_IO_IMPL,
+ HadoopFileIO.class.getName(),
+ HadoopFileIO.class,
+ new Class<?>[] { Configuration.class },
+ new Object[]{ conf });
Review comment:
The method supports no arg constructor, and also allows using multiple
constructor options. The main reason for refactoring is because it has 2
lengthy try catch blocks for NoSuchMethodException when looking for a
constructor, and also a ClassCastException when trying to cast the class.
But I agree that it loses the ability to check default implementation at
compile time. What if I leave the default class construction in the else
blocks, and keep the util to only initialize the dynamic class?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
