omarsmak commented on a change in pull request #3326:
URL: https://github.com/apache/iceberg/pull/3326#discussion_r733383409
##########
File path: core/src/main/java/org/apache/iceberg/CatalogUtil.java
##########
@@ -208,23 +201,14 @@ public static Catalog loadCatalog(
*/
public static Catalog buildIcebergCatalog(String name, Map<String, String>
options, Configuration conf) {
String catalogImpl = options.get(CatalogProperties.CATALOG_IMPL);
+ String catalogType = options.get(CatalogProperties.CATALOG_TYPE);
+ Preconditions.checkArgument(catalogImpl == null || catalogType == null,
+ "Cannot create catalog %s, both type and catalog-impl are set:
type=%s, catalog-impl=%s",
Review comment:
@kbendick I think that `type` in `CatalogUtil` is actually only for
Spark and Hive. For Flink, this needs to be adjusted in `FlinkCatalogFactory`
which will use `catalog-type` and thus, this PR doesn't really touch anything
related with Flink as @harshm-dev mentioned, this will come as follow up PR.
IMHO, I think this PR doesn't change any existing behavior, it just extends an
existing functionality and therefore, I don't see any issues here.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
