[GitHub] [gobblin] Will-Lo commented on a diff in pull request #3643: [GOBBLIN-1786] Support Other Catalog Types for Iceberg Distcp

["Will-Lo (via GitHub)" <gi...@apache.org>]

Will-Lo commented on code in PR #3643:
URL: https://github.com/apache/gobblin/pull/3643#discussion_r1114805445


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergCatalog.java:
##########
@@ -17,10 +17,25 @@
 
 package org.apache.gobblin.data.management.copy.iceberg;
 
+import org.apache.iceberg.catalog.Catalog;
+
 
 /**
  * Any catalog from which to access {@link IcebergTable}s.
  */
 public interface IcebergCatalog {
   IcebergTable openTable(String dbName, String tableName);
+  String getCatalogUri();
+
+  /**
+   * Adding a sub interface to help us provide an association between {@link Catalog} and {@link IcebergCatalog}.
+   * This helps us resolve to the Catalog to its concrete implementation class
+   * Primarily needed to access `newTableOps` method which only certain {@link Catalog} derived classes open for public access
+   */
+  interface CatalogSpecifier {

Review Comment:
   Yeah I'm also wondering on this, is this layer of abstraction necessary for future extensions? 



##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergDatasetFinder.java:
##########
@@ -21,19 +21,25 @@
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.Properties;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 
+import com.google.api.client.util.Maps;

Review Comment:
   is it possible to use new HashMap<>() and avoid this dep? 



-- 
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: dev-unsubscribe@gobblin.apache.org

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