ZihanLi58 commented on code in PR #3643:
URL: https://github.com/apache/gobblin/pull/3643#discussion_r1115162459


##########
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:
   If we are saying icebergCatalog's behavior is binding with the underlying 
catalog type, given that we already have one easy way 
(CatalogUtil.loadCatalog()) to load the underlying catalog easily, I think you 
can just keep with the original implementation, where you just specify the 
icebergCatalog type and then under icebergCatalog, you force the specific 
catalog (hiveCatalog for example). On IcebergMetataWriter side, we can just use 
CatalogUtil to load the catalog... 



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

Reply via email to