You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/07/01 17:29:44 UTC
[GitHub] [iceberg] kbendick commented on a diff in pull request #5045: Docs: Fix Flink Connector docs with custom catalog
kbendick commented on code in PR #5045:
URL: https://github.com/apache/iceberg/pull/5045#discussion_r912125559
##########
docs/flink-connector.md:
##########
@@ -35,10 +35,8 @@ To create the table in Flink SQL by using SQL syntax `CREATE TABLE test (..) WIT
* `connector`: Use the constant `iceberg`.
* `catalog-name`: User-specified catalog name. It's required because the connector don't have any default value.
-* `catalog-type`: Default to use `hive` if don't specify any value. The optional values are:
- * `hive`: The Hive metastore catalog.
- * `hadoop`: The hadoop catalog.
- * `custom`: The customized catalog, see [custom catalog](../custom-catalog) for more details.
+* `catalog-type`: `hive` or `hadoop` for built-in catalogs (defaults to `hive`), or left unset for custom catalog implementations using `catalog-impl`.
+* `catalog-impl`: The fully-qualified class name custom catalog implementation, must be set if `catalog-type` is unset. See also [custom catalog](../flink/flink-getting-started.md#custom-catalog) for more details.
Review Comment:
I agree it should be clear already that one or the other needs to be set from the language already.
##########
docs/flink-connector.md:
##########
@@ -107,7 +105,6 @@ CREATE TABLE flink_table (
) WITH (
'connector'='iceberg',
'catalog-name'='custom_prod',
- 'catalog-type'='custom',
Review Comment:
+1 to not having this configuration combination which can’t exist.
Should we also update the sentence on line 99 above which mentions that this “maps to the iceberg table `default_database.flink_table` managed in custom catalog.”? Specifically maybe mentioning that the table is “managed in a custom catalog of type com.my.custom.CatalogImpl” instead?
##########
docs/flink-connector.md:
##########
@@ -35,10 +35,8 @@ To create the table in Flink SQL by using SQL syntax `CREATE TABLE test (..) WIT
* `connector`: Use the constant `iceberg`.
* `catalog-name`: User-specified catalog name. It's required because the connector don't have any default value.
-* `catalog-type`: Default to use `hive` if don't specify any value. The optional values are:
- * `hive`: The Hive metastore catalog.
- * `hadoop`: The hadoop catalog.
- * `custom`: The customized catalog, see [custom catalog](../custom-catalog) for more details.
+* `catalog-type`: `hive` or `hadoop` for built-in catalogs (defaults to `hive`), or left unset for custom catalog implementations using `catalog-impl`.
+* `catalog-impl`: The fully-qualified class name custom catalog implementation, must be set if `catalog-type` is unset. See also [custom catalog](../flink/flink-getting-started.md#custom-catalog) for more details.
Review Comment:
Nit: “The fully qualified class name _of a_ custom catalog implementation”. The addition of `of a` makes the sentence a bit more fluent.
Then I’d start a new sentence for “Must be set if `catalog-type` is unset”. But that part might just be my own opinion, but it reads a bit more clearly to me.
--
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: issues-unsubscribe@iceberg.apache.org
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