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

Reply via email to