openinx commented on a change in pull request #1586:
URL: https://github.com/apache/iceberg/pull/1586#discussion_r509839531



##########
File path: site/docs/flink.md
##########
@@ -97,7 +97,21 @@ CREATE CATALOG hive_catalog WITH (
   'catalog-type'='hive',
   'uri'='thrift://localhost:9083',
   'clients'='5',
-  'property-version'='1'
+  'property-version'='1',
+  'hive-conf-dir'='/opt/hive/conf'
+);
+```
+
+Alternatively one can instead set just the `warehouse` property (without 
specifying a Hive configuration directory) to initialize the Hive catalog:

Review comment:
       Well, let me make this more clear. 

##########
File path: site/docs/flink.md
##########
@@ -293,4 +309,4 @@ There are some features that we do not yet support in the 
current flink iceberg
 * Don't support creating iceberg table with computed column.
 * Don't support creating iceberg table with watermark.
 * Don't support adding columns, removing columns, renaming columns, changing 
columns. [FLINK-19062](https://issues.apache.org/jira/browse/FLINK-19062) is 
tracking this.
-* Don't support flink read iceberg table in batch or streaming mode. 
[#1346](https://github.com/apache/iceberg/pull/1346) and 
[#1293](https://github.com/apache/iceberg/pull/1293) are tracking this. 
\ No newline at end of file
+* Don't support flink read iceberg table in batch or streaming mode. 
[#1346](https://github.com/apache/iceberg/pull/1346) and 
[#1293](https://github.com/apache/iceberg/pull/1293) are tracking this.

Review comment:
       Yes, that's right. 

##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
##########
@@ -72,13 +72,19 @@ public HiveCatalog(Configuration conf) {
   }
 
   public HiveCatalog(String name, String uri, int clientPoolSize, 
Configuration conf) {
+    this(name, uri, null, clientPoolSize, conf);
+  }
+
+  public HiveCatalog(String name, String uri, String warehouse, int 
clientPoolSize, Configuration conf) {
     this.name = name;
     this.conf = new Configuration(conf);
     // before building the client pool, overwrite the configuration's URIs if 
the argument is non-null
     if (uri != null) {
       this.conf.set(HiveConf.ConfVars.METASTOREURIS.varname, uri);
     }
-
+    if (warehouse != null) {
+      this.conf.set(HiveConf.ConfVars.METASTOREWAREHOUSE.varname, warehouse);

Review comment:
       As we discussed in this 
[comment](https://github.com/apache/iceberg/pull/1586/files#r509462458),   we 
could set `warehouse` string or `hive-conf-dir` to get the hive warehouse.   If 
use a local field,  then in getWarehouseLocation,  we would check the local 
field first and then read the hiveConf ?  I want to unify the parse path so I  
put this key-value into hiveConf (It's a deep-cloned conf, so should not affect 
the origin conf).




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