lirui-apache commented on code in PR #6698:
URL: https://github.com/apache/iceberg/pull/6698#discussion_r1091540668


##########
core/src/main/java/org/apache/iceberg/CatalogUtil.java:
##########
@@ -439,4 +439,40 @@ public static MetricsReporter loadMetricsReporter(String 
impl) {
 
     return reporter;
   }
+
+  /**
+   * Load a custom ClientPool implementation.
+   *
+   * <p>The ClientPool must have a no-arg constructor. {@link 
ClientPool#initialize(Map, Object)} is
+   * called to complete the initialization.
+   *
+   * @param impl ClientPool implementation full class name
+   * @param properties catalog properties
+   * @param conf hadoop configuration if needed
+   * @return initialized ClientPool object
+   * @throws IllegalArgumentException if no-arg constructor not found or error 
during initialization
+   */
+  public static <C, E extends Exception> ClientPool<C, E> loadClientPool(
+      String impl, Map<String, String> properties, Object conf) {
+    Preconditions.checkNotNull(
+        impl, "Cannot initialize custom ClientPool, impl class name is null");

Review Comment:
   IMO the util method here shouldn't care about where the impl class name come 
from. Maybe it's better to change the message like `Cannot initialize custom 
ClientPool, impl class name is null. Please check the value of 
CatalogProperties.CLIENT_POOL_IMPL.` Let me know if you think otherwise



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to