rdblue commented on a change in pull request #1875:
URL: https://github.com/apache/iceberg/pull/1875#discussion_r547439649
##########
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);
+ props.put(CatalogProperties.WAREHOUSE_LOCATION, warehouseLocation);
+ initialize(name, props);
+ }
+ @Override
+ public void initialize(String name, Map<String, String> properties) {
+ String inputWarehouseLocation =
properties.get(CatalogProperties.WAREHOUSE_LOCATION);
+ Preconditions.checkArgument(inputWarehouseLocation != null &&
!inputWarehouseLocation.equals(""),
+ "no location provided for warehouse");
Review comment:
Nit: Error messages should use sentence case, with the first word
capitalized. I'd also make it more clear that `warehouse` is the configuration
key to set, like "Cannot create Hadoop catalog without 'warehouse' location".
----------------------------------------------------------------
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]
