respond to code review comments

Author: Ryan Murray

nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java

@@ -53,6 +53,8 @@
 import org.apache.iceberg.io.FileIO;
 import org.apache.iceberg.relocated.com.google.common.base.Joiner;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Nessie implementation of Iceberg Catalog.
@@ -64,7 +66,7 @@
  * </p>
  */
 public class NessieCatalog extends BaseMetastoreCatalog implements AutoCloseable, SupportsNamespaces, Configurable {
-
+  private static final Logger logger = LoggerFactory.getLogger(NessieCatalog.class);
   private static final Joiner SLASH = Joiner.on("/");
   public static final String NESSIE_WAREHOUSE_DIR = "nessie.warehouse.dir";
   private NessieClient client;
@@ -74,9 +76,6 @@ public class NessieCatalog extends BaseMetastoreCatalog implements AutoCloseable
   private String name;
   private FileIO fileIO;
 
-  /**
-   * Try to avoid passing parameters via hadoop config. Dynamic catalog expects Map instead
-   */
   public NessieCatalog() {
   }
 
@@ -85,14 +84,13 @@ public void initialize(String inputName, Map<String, String> options) {
     String fileIOImpl = options.get(CatalogProperties.FILE_IO_IMPL);
     this.fileIO = fileIOImpl == null ? new HadoopFileIO(config) : CatalogUtil.loadFileIO(fileIOImpl, options, config);
     this.name = inputName == null ? "nessie" : inputName;
-    this.client = NessieClient.withConfig(s -> options.getOrDefault(s, config.get(s)));
+    this.client = NessieClient.withConfig(options::get);
 
-    this.warehouseLocation = options.getOrDefault(NESSIE_WAREHOUSE_DIR, config.get(NESSIE_WAREHOUSE_DIR));
+    this.warehouseLocation = options.get(NESSIE_WAREHOUSE_DIR);
     if (warehouseLocation == null) {
       throw new IllegalStateException("Parameter nessie.warehouse.dir not set, nessie can't store data.");
     }
-    final String requestedRef = options.getOrDefault(NessieClient.CONF_NESSIE_REF,
-        config.get(NessieClient.CONF_NESSIE_REF));
+    final String requestedRef = options.get(NessieClient.CONF_NESSIE_REF);
     this.reference = loadReference(requestedRef);
   }
 
@@ -148,11 +146,13 @@ public boolean dropTable(TableIdentifier identifier, boolean purge) {
       } catch (NessieConflictException e) {
         // pass for retry
       } catch (NessieNotFoundException e) {
-        throw new RuntimeException("Cannot drop table: ref is no longer valid.", e);
+        logger.error("Cannot drop table: ref is no longer valid.", e);
+        return false;
       }
     }
     if (count >= 5) {
-      throw new RuntimeException("Cannot drop table: failed after retry (update hash and retry)");
+      logger.error("Cannot drop table: failed after retry (update hash and retry)");
+      return false;
     }
     return true;
   }

nessie/src/main/java/org/apache/iceberg/nessie/NessieTableOperations.java

@@ -64,7 +64,7 @@ protected void doRefresh() {
     try {
       reference.refresh();
     } catch (NessieNotFoundException e) {
-      throw new RuntimeException("Failed to drop table as ref is no longer valid.", e);
+      throw new RuntimeException("Failed to refresh as ref is no longer valid.", e);
     }
     String metadataLocation = null;
     try {

nessie/src/test/java/org/apache/iceberg/nessie/BaseTestIceberg.java

@@ -61,6 +61,7 @@ public abstract class BaseTestIceberg {
   protected ContentsApi contents;
   protected Configuration hadoopConfig;
   protected final String branch;
+  private String path;
 
   public BaseTestIceberg(String branch) {
     this.branch = branch;
@@ -80,7 +81,7 @@ private void resetData() throws NessieConflictException, NessieNotFoundException
   @Before
   public void beforeEach() throws IOException {
     String port = System.getProperty("quarkus.http.test-port", "19120");
-    String path = String.format("http://localhost:%s/api/v1", port);
+    path = String.format("http://localhost:%s/api/v1", port);
     this.client = NessieClient.none(path);
     tree = client.getTreeApi();
     contents = client.getContentsApi();
@@ -94,17 +95,17 @@ public void beforeEach() throws IOException {
     }
 
     hadoopConfig = new Configuration();
-    hadoopConfig.set(NessieClient.CONF_NESSIE_URL, path);
-    hadoopConfig.set(NessieClient.CONF_NESSIE_REF, branch);
-    hadoopConfig.set(NessieClient.CONF_NESSIE_AUTH_TYPE, "NONE");
-    hadoopConfig.set("nessie.warehouse.dir", temp.getRoot().toURI().toString());
     catalog = initCatalog(branch);
   }
 
   NessieCatalog initCatalog(String ref) {
     NessieCatalog newCatalog = new NessieCatalog();
     newCatalog.setConf(hadoopConfig);
-    newCatalog.initialize(null, ImmutableMap.of(NessieClient.CONF_NESSIE_REF, ref));
+    newCatalog.initialize(null, ImmutableMap.of(NessieClient.CONF_NESSIE_REF, ref,
+        NessieClient.CONF_NESSIE_URL, path,
+        NessieClient.CONF_NESSIE_AUTH_TYPE, "NONE",
+        "nessie.warehouse.dir", temp.getRoot().toURI().toString()
+        ));
     return newCatalog;
   }