kevinjqliu commented on code in PR #2175: URL: https://github.com/apache/iceberg-python/pull/2175#discussion_r2191469681
########## mkdocs/docs/configuration.md: ########## @@ -339,25 +339,19 @@ catalog: | Key | Example | Description | | ------------------- | -------------------------------- | -------------------------------------------------------------------------------------------------- | -| uri | <https://rest-catalog/ws> | URI identifying the REST Server | +| uri | <https://rest-catalog/ws> | URI identifying the REST Server | | ugi | t-1234:secret | Hadoop UGI for Hive client. | -| credential | t-1234:secret | Credential to use for OAuth2 credential flow when initializing the catalog | -| token | FEW23.DFSDF.FSDF | Bearer token value to use for `Authorization` header | | scope | openid offline corpds:ds:profile | Desired scope of the requested security token (default : catalog) | | resource | rest_catalog.iceberg.com | URI for the target resource or service | | audience | rest_catalog | Logical name of target resource or service | -| rest.sigv4-enabled | true | Sign requests to the REST Server using AWS SigV4 protocol | -| rest.signing-region | us-east-1 | The region to use when SigV4 signing a request | -| rest.signing-name | execute-api | The service signing name to use when SigV4 signing a request | -| oauth2-server-uri | <https://auth-service/cc> | Authentication URL to use for client credentials authentication (default: uri + 'v1/oauth/tokens') | -| snapshot-loading-mode | refs | The snapshots to return in the body of the metadata. Setting the value to `all` would return the full set of snapshots currently valid for the table. Setting the value to `refs` would load all snapshots referenced by branches or tags. | -| warehouse | myWarehouse | Warehouse location or identifier to request from the catalog service. May be used to determine server-side overrides, such as the warehouse location. | +| snapshot-loading-mode | refs | The snapshots to return in the body of the metadata. Setting the value to `all` would return the full set of snapshots currently valid for the table. Setting the value to `refs` would load all snapshots referenced by branches or tags. | +| warehouse | myWarehouse | Warehouse location or identifier to request from the catalog service. May be used to determine server-side overrides, such as the warehouse location. | Review Comment: nit: move this under `uri` since this is important :) ########## mkdocs/docs/configuration.md: ########## @@ -368,11 +362,84 @@ catalog: header.content-type: application/vnd.api+json ``` -Specific headers defined by the RESTCatalog spec include: -| Key | Options | Default | Description | -| ------------------------------------ | ------------------------------------- | -------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `header.X-Iceberg-Access-Delegation` | `{vended-credentials,remote-signing}` | `vended-credentials` | Signal to the server that the client supports delegated access via a comma-separated list of access mechanisms. The server may choose to supply access via any or none of the requested mechanisms | +#### Authentication Options +- **SigV4**: For AWS services that require SigV4 signing. +- **OAuth2**: For services that require OAuth2 authentication. Review Comment: lets add these as 2 separate subsections. One for OAuth2, followed by SigV4. OAuth2 has options are `oauth2-server-uri`, `token`, `credential`, along with the few from my comment on top SigV4 is an AWS specific protocol. these are all related to sigv4:`rest.sigv4-enabled`, `rest.signing-region`, `rest.signing-name` ########## mkdocs/docs/configuration.md: ########## @@ -339,25 +339,19 @@ catalog: | Key | Example | Description | | ------------------- | -------------------------------- | -------------------------------------------------------------------------------------------------- | -| uri | <https://rest-catalog/ws> | URI identifying the REST Server | +| uri | <https://rest-catalog/ws> | URI identifying the REST Server | | ugi | t-1234:secret | Hadoop UGI for Hive client. | Review Comment: `ugi` should actually not be in REST catalog. its a hive catalog option that was accidentally added here https://github.com/apache/iceberg-python/commit/5209874cd4685427520b67b64188f5e9919f2816#diff-497e037708cc64870c6ba9372f6064a69ca1e74d65d6195dcee5a44851e8b47dR151 ########## mkdocs/docs/configuration.md: ########## @@ -339,25 +339,19 @@ catalog: | Key | Example | Description | | ------------------- | -------------------------------- | -------------------------------------------------------------------------------------------------- | -| uri | <https://rest-catalog/ws> | URI identifying the REST Server | +| uri | <https://rest-catalog/ws> | URI identifying the REST Server | | ugi | t-1234:secret | Hadoop UGI for Hive client. | -| credential | t-1234:secret | Credential to use for OAuth2 credential flow when initializing the catalog | -| token | FEW23.DFSDF.FSDF | Bearer token value to use for `Authorization` header | | scope | openid offline corpds:ds:profile | Desired scope of the requested security token (default : catalog) | | resource | rest_catalog.iceberg.com | URI for the target resource or service | | audience | rest_catalog | Logical name of target resource or service | Review Comment: move these 3 to the OAuth section since these are oauth concepts https://github.com/apache/iceberg-python/blob/d48a08d2da96088524211f1ef9607e9a8c34ca46/pyiceberg/catalog/rest/__init__.py#L334-L335 -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org