You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2023/04/26 11:27:49 UTC

[GitHub] [iceberg] nastra commented on a diff in pull request #7433: Update documentation to reflect new catalog features

nastra commented on code in PR #7433:
URL: https://github.com/apache/iceberg/pull/7433#discussion_r1177734641


##########
docs/spark-configuration.md:
##########
@@ -63,15 +63,20 @@ Iceberg supplies two implementations:
 
 Both catalogs are configured using properties nested under the catalog name. Common configuration properties for Hive and Hadoop are:
 
-| Property                                           | Values                        | Description                                                          |
-| -------------------------------------------------- | ----------------------------- | -------------------------------------------------------------------- |
-| spark.sql.catalog._catalog-name_.type              | `hive`, `hadoop` or `rest`    | The underlying Iceberg catalog implementation, `HiveCatalog`, `HadoopCatalog`, `RESTCatalog` or left unset if using a custom catalog |
-| spark.sql.catalog._catalog-name_.catalog-impl      |                               | The underlying Iceberg catalog implementation.|
-| spark.sql.catalog._catalog-name_.default-namespace | default                       | The default current namespace for the catalog |
-| spark.sql.catalog._catalog-name_.uri               | thrift://host:port            | Metastore connect URI; default from `hive-site.xml` |
-| spark.sql.catalog._catalog-name_.warehouse         | hdfs://nn:8020/warehouse/path | Base path for the warehouse directory |
-| spark.sql.catalog._catalog-name_.cache-enabled     | `true` or `false`             | Whether to enable catalog cache, default value is `true` |
-| spark.sql.catalog._catalog-name_.cache.expiration-interval-ms | `30000` (30 seconds) | Duration after which cached catalog entries are expired; Only effective if `cache-enabled` is `true`. `-1` disables cache expiration and `0` disables caching entirely, irrespective of `cache-enabled`. Default is `30000` (30 seconds) |                                                   |
+| Property                                                      | Values                        | Description                                                                                                                                                                                                                              |
+|---------------------------------------------------------------|-------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| spark.sql.catalog._catalog-name_.type                         | `hive`, `hadoop` or `rest`    | The underlying Iceberg catalog implementation, `HiveCatalog`, `HadoopCatalog`, `RESTCatalog` or left unset if using a custom catalog                                                                                                     |

Review Comment:
   do we really need to adjust the header separator `----------------------------------------------|` and also the whitespace for each row as it adds unnecessary diffs and makes it more difficult to review?



##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -89,16 +89,22 @@
  * <p>This supports the following catalog configuration options:
  *
  * <ul>
- *   <li><code>type</code> - catalog type, "hive" or "hadoop". To specify a non-hive or hadoop
- *       catalog, use the <code>catalog-impl</code> option.
- *   <li><code>uri</code> - the Hive Metastore URI (Hive catalog only)
+ *   <li><code>type</code> - catalog type, "hive" or "hadoop" or "rest". To specify a non-hive or
+ *       hadoop catalog, use the <code>catalog-impl</code> option.
+ *   <li><code>uri</code> - the Hive Metastore URI for Hive catalog or REST URI for rest catalog

Review Comment:
   ```suggestion
    *   <li><code>uri</code> - the Hive Metastore URI for Hive catalog or REST URI for REST catalog
   ```



##########
docs/spark-configuration.md:
##########
@@ -63,15 +63,20 @@ Iceberg supplies two implementations:
 
 Both catalogs are configured using properties nested under the catalog name. Common configuration properties for Hive and Hadoop are:
 
-| Property                                           | Values                        | Description                                                          |
-| -------------------------------------------------- | ----------------------------- | -------------------------------------------------------------------- |
-| spark.sql.catalog._catalog-name_.type              | `hive`, `hadoop` or `rest`    | The underlying Iceberg catalog implementation, `HiveCatalog`, `HadoopCatalog`, `RESTCatalog` or left unset if using a custom catalog |
-| spark.sql.catalog._catalog-name_.catalog-impl      |                               | The underlying Iceberg catalog implementation.|
-| spark.sql.catalog._catalog-name_.default-namespace | default                       | The default current namespace for the catalog |
-| spark.sql.catalog._catalog-name_.uri               | thrift://host:port            | Metastore connect URI; default from `hive-site.xml` |
-| spark.sql.catalog._catalog-name_.warehouse         | hdfs://nn:8020/warehouse/path | Base path for the warehouse directory |
-| spark.sql.catalog._catalog-name_.cache-enabled     | `true` or `false`             | Whether to enable catalog cache, default value is `true` |
-| spark.sql.catalog._catalog-name_.cache.expiration-interval-ms | `30000` (30 seconds) | Duration after which cached catalog entries are expired; Only effective if `cache-enabled` is `true`. `-1` disables cache expiration and `0` disables caching entirely, irrespective of `cache-enabled`. Default is `30000` (30 seconds) |                                                   |
+| Property                                                      | Values                        | Description                                                                                                                                                                                                                              |
+|---------------------------------------------------------------|-------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| spark.sql.catalog._catalog-name_.type                         | `hive`, `hadoop` or `rest`    | The underlying Iceberg catalog implementation, `HiveCatalog`, `HadoopCatalog`, `RESTCatalog` or left unset if using a custom catalog                                                                                                     |
+| spark.sql.catalog._catalog-name_.catalog-impl                 |                               | The underlying Iceberg catalog implementation.                                                                                                                                                                                           |
+| spark.sql.catalog._catalog-name_.io-impl                      |                               | The custom FileIO implementation.                                                                                                                                                                                                        |
+| spark.sql.catalog._catalog-name_.metrics-reporter-impl        |                               | The custom MetricsReporter implementation.                                                                                                                                                                                               |
+| spark.sql.catalog._catalog-name_.default-namespace            | default                       | The default current namespace for the catalog                                                                                                                                                                                            |
+| spark.sql.catalog._catalog-name_.uri                          | thrift://host:port            | Hive Metastore URI (default from `hive-site.xml`) for hive catalog and REST URI for rest catalog                                                                                                                                         |

Review Comment:
   ```suggestion
   | spark.sql.catalog._catalog-name_.uri                          | thrift://host:port            | Hive Metastore URI (default from `hive-site.xml`) for hive catalog and REST URI for REST catalog                                                                                                                                         |
   ```



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