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