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