yyanyy commented on a change in pull request #2146:
URL: https://github.com/apache/iceberg/pull/2146#discussion_r564199952
##########
File path: core/src/test/java/org/apache/iceberg/TestCatalogUtil.java
##########
@@ -87,6 +87,32 @@ public void loadCustomCatalog_NotImplementCatalog() {
() -> CatalogUtil.loadCatalog(TestCatalogNoInterface.class.getName(),
name, options, hadoopConf));
}
+ @Test
+ public void loadCustomCatalog_ConstructorErrorCatalog() {
+ Map<String, String> options = new HashMap<>();
+ options.put("key", "val");
+ Configuration hadoopConf = new Configuration();
+ String name = "custom";
+
+ String impl = TestCatalogErrorConstructor.class.getName();
+ AssertHelpers.assertThrows("must be able to initialize catalog",
Review comment:
Nit: "must not be able to"?
##########
File path: core/src/main/java/org/apache/iceberg/CatalogUtil.java
##########
@@ -129,6 +129,35 @@ private static void deleteFiles(FileIO io,
Set<ManifestFile> allManifests) {
});
}
+ /**
+ * Get constructor for Catalog from impl. Impl must be valid class name and
have zero-arg constructor.
+ *
+ * <p>We try to get the constructor for impl and if we fail we check for
common error patterns. Specifically:
+ * a ClassNotFoundException for an invalid impl String and a
NoClassDefFoundError if impl fails to get initialized.
+ * Otherwise the original exception is passed up and the error is assumed to
be a missing no-args constructor.
+ *
+ * @param impl catalog implementation full class name
+ * @return constructor for catalog implementation impl
+ * @throws IllegalArgumentException if no-arg constructor not found or error
during initialization
+ */
+ private static DynConstructors.Ctor<Catalog> getCatalogConstructor(String
impl) {
+ DynConstructors.Ctor<Catalog> ctor;
+ DynConstructors.Builder ctorBuilder =
DynConstructors.builder(Catalog.class).impl(impl);
+ try {
+ ctor = ctorBuilder.buildChecked();
+ } catch (NoSuchMethodException e) {
+ Throwable originalError = ctorBuilder.problems().getOrDefault(impl, e);
+ String message = String.format("Cannot initialize Catalog, missing
no-arg constructor: %s", impl);
Review comment:
Nit: "Cannot initialize Catalog" will be printed twice if the error is
neither of the one below. Although I wonder if there's a specific exception
thrown if the catalog class can be found but does not have a no-arg
constructor, and if so we probably could catch and print that instead, since
I'd suspect the chance of misconfiguring could be much larger than only missing
the no-arg constructor itself, and hence "cannot be initialized" might be a
better default ...
----------------------------------------------------------------
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]
