roryqi commented on code in PR #11113:
URL: https://github.com/apache/gravitino/pull/11113#discussion_r3246801407


##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java:
##########
@@ -105,15 +111,42 @@ protected LanceTableOperations newTableOps() {
 
   @Override
   public void close() {
-    if (client != null) {
+    if (catalogFetcher != null) {
       try {
-        client.close();
+        catalogFetcher.close();
       } catch (Exception e) {
-        LOG.warn("Error closing Gravitino client", e);
+        LOG.warn("Error closing Lance catalog fetcher", e);
       }
     }
   }
 
+  Catalog[] listCatalogsInfo() throws NoSuchMetalakeException {
+    return catalogFetcher.listCatalogsInfo();
+  }
+
+  Catalog loadCatalog(String catalogName) throws NoSuchCatalogException {
+    return catalogFetcher.loadCatalog(catalogName);
+  }
+
+  Catalog createCatalog(

Review Comment:
   Why do we need to `createCatalog` for catalogFetcher?



##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNameSpaceOperations.java:
##########


Review Comment:
   I don't like this design. Schema should align the namespace instead of 
`Catalog + schema`. cc @yuqi1129 @FANNG1 



##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java:
##########
@@ -105,15 +111,42 @@ protected LanceTableOperations newTableOps() {
 
   @Override
   public void close() {
-    if (client != null) {
+    if (catalogFetcher != null) {
       try {
-        client.close();
+        catalogFetcher.close();
       } catch (Exception e) {
-        LOG.warn("Error closing Gravitino client", e);
+        LOG.warn("Error closing Lance catalog fetcher", e);
       }
     }
   }
 
+  Catalog[] listCatalogsInfo() throws NoSuchMetalakeException {
+    return catalogFetcher.listCatalogsInfo();
+  }
+
+  Catalog loadCatalog(String catalogName) throws NoSuchCatalogException {
+    return catalogFetcher.loadCatalog(catalogName);
+  }
+
+  Catalog createCatalog(

Review Comment:
   If we need to create the Lance catalog, catalog fetcher isn't a good name. 
Fetcher means that read data instead of writing data.



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