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:
[email protected]



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

Reply via email to