rdblue commented on a change in pull request #1875:
URL: https://github.com/apache/iceberg/pull/1875#discussion_r547438609



##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java
##########
@@ -68,45 +70,62 @@
  *
  * Note: The HadoopCatalog requires that the underlying file system supports 
atomic rename.
  */
-public class HadoopCatalog extends BaseMetastoreCatalog implements Closeable, 
SupportsNamespaces {
+public class HadoopCatalog extends BaseMetastoreCatalog implements Closeable, 
SupportsNamespaces, Configurable {
 
   private static final String ICEBERG_HADOOP_WAREHOUSE_BASE = 
"iceberg/warehouse";
   private static final String TABLE_METADATA_FILE_EXTENSION = ".metadata.json";
   private static final Joiner SLASH = Joiner.on("/");
   private static final PathFilter TABLE_FILTER = path -> 
path.getName().endsWith(TABLE_METADATA_FILE_EXTENSION);
 
-  private final String catalogName;
-  private final Configuration conf;
-  private final String warehouseLocation;
-  private final FileSystem fs;
-  private final FileIO fileIO;
+  private String catalogName;
+  private Configuration conf;
+  private String warehouseLocation;
+  private FileSystem fs;
+  private FileIO fileIO;
+
+  public HadoopCatalog(){
+  }
 
   /**
    * The constructor of the HadoopCatalog. It uses the passed location as its 
warehouse directory.
    *
+   * @deprecated please use the no-arg constructor, setConf and initialize to 
construct the catalog
    * @param name The catalog name
    * @param conf The Hadoop configuration
    * @param warehouseLocation The location used as warehouse directory
    */
+  @Deprecated
   public HadoopCatalog(String name, Configuration conf, String 
warehouseLocation) {
     this(name, conf, warehouseLocation, Maps.newHashMap());
   }
 
   /**
    * The all-arg constructor of the HadoopCatalog.
    *
+   * @deprecated please use the no-arg constructor, setConf and initialize to 
construct the catalog
    * @param name The catalog name
    * @param conf The Hadoop configuration
    * @param warehouseLocation The location used as warehouse directory
    * @param properties catalog properties
    */
+  @Deprecated
   public HadoopCatalog(String name, Configuration conf, String 
warehouseLocation, Map<String, String> properties) {
     Preconditions.checkArgument(warehouseLocation != null && 
!warehouseLocation.equals(""),
         "no location provided for warehouse");
+    setConf(conf);
+    Map<String, String> props = new HashMap<>(properties);
+    props.putAll(properties);

Review comment:
       This adds properties to the map twice?
   
   We usually prefer `Maps.newHashMap()`, too.




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to