jackye1995 commented on a change in pull request #3590:
URL: https://github.com/apache/iceberg/pull/3590#discussion_r754792952
##########
File path: core/src/main/java/org/apache/iceberg/CatalogUtil.java
##########
@@ -152,8 +154,7 @@ private static void deleteFiles(FileIO io,
Set<ManifestFile> allManifests) {
* Load a custom catalog implementation.
* <p>
* The catalog must have a no-arg constructor.
- * If the class implements {@link Configurable},
- * a Hadoop config will be passed using {@link
Configurable#setConf(Configuration)}.
+ * If the class implements Configurable, a Hadoop config will be passed
using Configurable.setConf.
Review comment:
why the links are removed in javadoc?
##########
File path: core/src/main/java/org/apache/iceberg/CatalogUtil.java
##########
@@ -266,11 +264,76 @@ public static FileIO loadFileIO(
String.format("Cannot initialize FileIO, %s does not implement
FileIO.", impl), e);
}
- if (fileIO instanceof Configurable) {
- ((Configurable) fileIO).setConf(hadoopConf);
- }
+ configureHadoopConf(fileIO, hadoopConf);
fileIO.initialize(properties);
return fileIO;
}
+
+ /**
+ * Dynamically detects whether an object is a Hadoop Configurable and calls
setConf.
+ * @param maybeConfigurable an object that may be Configurable
+ * @param conf a Configuration
+ */
+ @SuppressWarnings("unchecked")
+ public static void configureHadoopConf(Object maybeConfigurable, Object
conf) {
+ Preconditions.checkArgument(maybeConfigurable != null, "Cannot configure:
null Configurable");
+ if (conf == null) {
+ return;
+ }
+
+ if (maybeConfigurable instanceof Configurable) {
+ // use the Iceberg configurable interface to pass the conf
+ ((Configurable<Object>) maybeConfigurable).setConf(conf);
+ return;
+ }
+
+ // try to use Hadoop's Configurable interface dynamically
+ // use the classloader of the object that may be configurable
+ ClassLoader maybeConfigurableLoader =
maybeConfigurable.getClass().getClassLoader();
+
+ Class<?> configurableInterface;
+ try {
+ // load the Configurable interface
+ configurableInterface = DynClasses.builder()
+ .loader(maybeConfigurableLoader)
+ .impl("org.apache.hadoop.conf.Configurable")
+ .buildChecked();
+ } catch (ClassNotFoundException e) {
+ // not Configurable because it was loaded and Configurable is not
present in its classloader
+ return;
+ }
+
+ if (!configurableInterface.isInstance(maybeConfigurable)) {
+ // not Configurable because the object does not implement the
Configurable interface
+ return;
+ }
+
+ Class<?> configurationClass;
+ try {
+ configurationClass = DynClasses.builder()
+ .loader(maybeConfigurableLoader)
+ .impl("org.apache.hadoop.conf.Configuration")
+ .buildChecked();
+ } catch (ClassNotFoundException e) {
+ // this shouldn't happen because Configurable cannot be loaded without
first loading Configuration
+ throw new UnsupportedOperationException("Failed to load Configuration
after loading Configurable", e);
Review comment:
should we use the full name `org.apache.hadoop.conf.Configuration`
instead of just `Configuration` in the error message? (similarly for the other
exception messages below)
##########
File path: core/src/main/java/org/apache/iceberg/CatalogUtil.java
##########
@@ -266,11 +264,76 @@ public static FileIO loadFileIO(
String.format("Cannot initialize FileIO, %s does not implement
FileIO.", impl), e);
}
- if (fileIO instanceof Configurable) {
- ((Configurable) fileIO).setConf(hadoopConf);
- }
+ configureHadoopConf(fileIO, hadoopConf);
fileIO.initialize(properties);
return fileIO;
}
+
+ /**
+ * Dynamically detects whether an object is a Hadoop Configurable and calls
setConf.
+ * @param maybeConfigurable an object that may be Configurable
+ * @param conf a Configuration
+ */
+ @SuppressWarnings("unchecked")
+ public static void configureHadoopConf(Object maybeConfigurable, Object
conf) {
Review comment:
does this need to be public?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
