FANNG1 commented on code in PR #4273:
URL: https://github.com/apache/gravitino/pull/4273#discussion_r1697157752


##########
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/ConfigIcebergTableOpsProvider.java:
##########
@@ -0,0 +1,74 @@
+package org.apache.gravitino.iceberg.common.ops;
+
+import com.google.common.collect.Maps;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.apache.commons.lang.StringUtils;
+import org.apache.gravitino.iceberg.common.IcebergConfig;
+import org.apache.gravitino.utils.MapUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ConfigIcebergTableOpsProvider implements IcebergTableOpsProvider {

Review Comment:
   provide a config example?



##########
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergTableOpsManager.java:
##########
@@ -0,0 +1,117 @@
+package org.apache.gravitino.iceberg.common.ops;
+
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.apache.commons.lang.StringUtils;
+import org.apache.gravitino.iceberg.common.IcebergConfig;
+import org.apache.gravitino.utils.IsolatedClassLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class IcebergTableOpsManager implements AutoCloseable {
+  public static final Logger LOG = 
LoggerFactory.getLogger(IcebergTableOpsManager.class);
+
+  public static final String DEFAULT_CATALOG = "default_catalog";
+
+  private static final Splitter splitter = Splitter.on(",");
+
+  private final Map<String, IcebergTableOps> icebergTableOpsMap;
+
+  private final IcebergTableOpsProvider provider;
+
+  public IcebergTableOpsManager(IcebergConfig config) {
+    this.icebergTableOpsMap = Maps.newConcurrentMap();
+    this.provider = createProvider(config);
+    this.provider.initialize(config);
+  }
+
+  public IcebergTableOpsManager(IcebergTableOpsProvider provider) {
+    this.icebergTableOpsMap = Maps.newConcurrentMap();
+    this.provider = provider;
+  }
+
+  public IcebergTableOps getOps(String rawPrefix) {
+    String prefix = shelling(rawPrefix);
+    String cacheKey = prefix;
+    if (StringUtils.isBlank(prefix)) {
+      LOG.debug("prefix is empty, return default iceberg catalog");
+      cacheKey = DEFAULT_CATALOG;
+    }
+    return icebergTableOpsMap.computeIfAbsent(cacheKey, k -> 
provider.getIcebergTableOps(prefix));
+  }
+
+  public Optional<String> getPrefix(String rawPrefix, String warehouse) {
+    String prefix = shelling(rawPrefix);
+    if (!StringUtils.isBlank(prefix)) {
+      return Optional.of(prefix);
+    } else {
+      return provider.getPrefix(warehouse);
+    }
+  }
+
+  private IcebergTableOpsProvider createProvider(IcebergConfig config) {
+    try (IsolatedClassLoader isolatedClassLoader =

Review Comment:
   why use a separate classloader?



##########
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergTableOpsProvider.java:
##########
@@ -0,0 +1,28 @@
+package org.apache.gravitino.iceberg.common.ops;
+
+import java.util.Optional;
+import org.apache.gravitino.iceberg.common.IcebergConfig;
+
+/**
+ * IcebergTableOpsProvider is an interface defining how iceberg rest catalog 
server gets iceberg
+ * catalogs.
+ */
+public interface IcebergTableOpsProvider {
+
+  /** @param config The configuration parameters for creating Provider. */
+  void initialize(IcebergConfig config);
+
+  /**
+   * @param prefix the path param send by clients.
+   * @return the instance of IcebergTableOps.
+   */
+  IcebergTableOps getIcebergTableOps(String prefix);
+
+  /**
+   * Get a path prefix using by clients.
+   *
+   * @param warehouse the identifier for an iceberg catalog.
+   * @return a path prefix.
+   */
+  Optional<String> getPrefix(String warehouse);

Review Comment:
   `warehouse` is a sync properties for client and server,  seems not related 
to `prefix`, see https://github.com/apache/iceberg/pull/6150 



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