rdblue commented on a change in pull request #2146:
URL: https://github.com/apache/iceberg/pull/2146#discussion_r564744618



##########
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) {

Review comment:
       Nit: use of `get` when a more descriptive verb would be better. For 
example, `find` implies that it may not be found.




----------------------------------------------------------------
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]

Reply via email to