RussellSpitzer commented on a change in pull request #2333:
URL: https://github.com/apache/iceberg/pull/2333#discussion_r594410343
##########
File path: core/src/main/java/org/apache/iceberg/CatalogUtil.java
##########
@@ -215,16 +216,27 @@ public static FileIO loadFileIO(
Configuration hadoopConf) {
LOG.info("Loading custom FileIO implementation: {}", impl);
DynConstructors.Ctor<FileIO> ctor;
+ boolean useNoArgConstructor = true;
try {
ctor = DynConstructors.builder(FileIO.class).impl(impl).buildChecked();
} catch (NoSuchMethodException e) {
- throw new IllegalArgumentException(String.format(
- "Cannot initialize FileIO, missing no-arg constructor: %s", impl),
e);
+ LOG.error("No-arg constructor not found for {}, try to use Hadoop
Configuration constructor", impl, e);
+ try {
+ ctor = DynConstructors.builder(Catalog.class).impl(impl,
Configuration.class).buildChecked();
+ useNoArgConstructor = false;
Review comment:
Could we just invoke the constructor where we load it and skip the state
variable passing through here?
Something like
```java
try {
ctor = ..
fileIO = ctor.newInstance
} catch {
ctor = ... single are ctor
fileIo = ctor.newInstance(hadoopConf)
}
```
To avoid try nesting we could extract that to a helper function and then
have something like
```java
try {
fileIO = loadFileIO(impl)
} catch {
Cannot find a 0-arg or 1-arg constructor to initalize FileImplemenation
}
```
----------------------------------------------------------------
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]
