[GitHub] [iceberg] aokolnychyi commented on a change in pull request #2203: HiveCatalog and HiveClientPool fixes

[GitBox <gi...@apache.org>]

aokolnychyi commented on a change in pull request #2203:
URL: https://github.com/apache/iceberg/pull/2203#discussion_r592856169



##########
File path: flink/src/main/java/org/apache/iceberg/flink/CatalogLoader.java
##########
@@ -109,7 +109,7 @@ private HiveCatalogLoader(String catalogName, Configuration conf, Map<String, St
 
     @Override
     public Catalog loadCatalog() {
-      return new HiveCatalog(catalogName, uri, warehouse, clientPoolSize, hadoopConf.get(), properties);
+      return CatalogUtil.loadCatalog(HiveCatalog.class.getName(), catalogName, properties, hadoopConf.get());

Review comment:
       Question: Shall we use a constant in `CatalogUtil` for this? What way should we promote? Static import for the constant may make the line a bit shorter.

##########
File path: flink/src/test/java/org/apache/iceberg/flink/FlinkTestBase.java
##########
@@ -57,7 +59,8 @@ public static void startMetastore() {
     FlinkTestBase.metastore = new TestHiveMetastore();
     metastore.start();
     FlinkTestBase.hiveConf = metastore.hiveConf();
-    FlinkTestBase.catalog = new HiveCatalog(metastore.hiveConf());
+    FlinkTestBase.catalog = (HiveCatalog)

Review comment:
       I wish we parameterized `loadCatalog`.

##########
File path: spark/src/test/java/org/apache/iceberg/spark/SparkTestBase.java
##########
@@ -62,7 +64,8 @@ public static void startMetastoreAndSpark() {
         .enableHiveSupport()
         .getOrCreate();
 
-    SparkTestBase.catalog = new HiveCatalog(spark.sessionState().newHadoopConf());
+    SparkTestBase.catalog = (HiveCatalog)
+        CatalogUtil.loadCatalog(HiveCatalog.class.getName(), "hive", ImmutableMap.of(), hiveConf);

Review comment:
       nit: `hiveConf` is not technically the same as `sessionState().newHadoopConf()` but it does not matter in tests.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org