sungwy commented on code in PR #1033:
URL: https://github.com/apache/iceberg-python/pull/1033#discussion_r1722545650
##########
pyiceberg/catalog/rest.py:
##########
@@ -532,7 +534,7 @@ def _config_headers(self, session: Session) -> None:
session.headers["Content-type"] = "application/json"
session.headers["X-Client-Version"] = ICEBERG_REST_SPEC_VERSION
session.headers["User-Agent"] = f"PyIceberg/{__version__}"
- session.headers["X-Iceberg-Access-Delegation"] = "vended-credentials"
+ session.headers["X-Iceberg-Access-Delegation"] =
self.properties.get(ACCESS_DELEGATION, ACCESS_DELEGATION_DEFAULT)
Review Comment:
It looks like we already have a way of setting custom headers in
`_extract_headers_from_properties`. If I understand it correctly, if we set a
property like: `header.X-Iceberg-Access-Delegation = remote-signing` then this
should set the header "X-Iceberg-Access-Delegation" as `remote-signing`.
I think we could achieve this by setting the default header values, and then
setting the property based values after the default values are set:
```
session.headers["Content-type"] = "application/json"
session.headers["X-Client-Version"] = ICEBERG_REST_SPEC_VERSION
session.headers["User-Agent"] = f"PyIceberg/{__version__}"
session.headers["X-Iceberg-Access-Delegation"] = "vended-credentials"
header_properties = self._extract_headers_from_properties()
session.headers.update(header_properties)
```
What do you think of this approach over introducing a different property?
##########
mkdocs/docs/configuration.md:
##########
@@ -198,19 +198,20 @@ catalog:
<!-- markdown-link-check-disable -->
-| Key | Example | Description
|
-| ------------------- | -------------------------------- |
--------------------------------------------------------------------------------------------------
|
-| 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')
|
+| Key | Example | Description
|
+| ------------------- | -------------------------------- |
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
+| 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
|
+| access-delegation | remote-signing | A comma-separated
list of access mechanisms to signal the server that the client supports
delegated access. It will to be sended in `X-Iceberg-Access-Delegation` header.
(default: vended-credentials) |
Review Comment:
nit:
```suggestion
| access-delegation | remote-signing | A comma-separated
list of access mechanisms to signal the server that the client supports
delegated access. It will be sent in `X-Iceberg-Access-Delegation` header.
(default: vended-credentials) |
```
--
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]
