geruh commented on code in PR #3500:
URL: https://github.com/apache/iceberg-python/pull/3500#discussion_r3447585115
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -486,6 +488,40 @@ def _resolve_storage_credentials(storage_credentials:
list[StorageCredential], l
return best_match.config if best_match else {}
+ def _should_load_credentials_from_endpoint(self) -> bool:
+ if Capability.V1_LOAD_CREDENTIALS not in self._supported_endpoints:
+ return False
+
+ access_delegation = cast(str,
self._session.headers.get(ACCESS_DELEGATION_HEADER, ""))
+ # The spec encodes access delegation as a comma-separated list of
mechanisms.
+ # Load credentials only when vended-credentials is requested.
+ return any(delegation.strip().lower() == ACCESS_DELEGATION_DEFAULT for
delegation in access_delegation.split(","))
+
+ def _resolve_table_credentials_from_response_or_endpoint(
Review Comment:
Compared this to Java `RESTSessionCatalog` and `VendedCredentialsProvider`.
They don't call the `/credentials` api during `loadTable` and instead seems to
rely on inline `storage-credentials` on load_table call, and uses the
credential endpoint for refresh in FileIO.
We don't have that layer, today so this eager fallback makes sense to me.
And just putting this here so it's clear.
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -486,6 +488,40 @@ def _resolve_storage_credentials(storage_credentials:
list[StorageCredential], l
return best_match.config if best_match else {}
+ def _should_load_credentials_from_endpoint(self) -> bool:
+ if Capability.V1_LOAD_CREDENTIALS not in self._supported_endpoints:
+ return False
+
+ access_delegation = cast(str,
self._session.headers.get(ACCESS_DELEGATION_HEADER, ""))
Review Comment:
nit: do we need to cast or does it break linter? locally it shows
`unnecessary cast; type is already 'str'`
##########
pyiceberg/catalog/rest/__init__.py:
##########
Review Comment:
Looks like we are still inlining here still and not using
`_resolve_table_credentials_from_response_or_endpoint`, or are you handling
that as a followup?
--
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]
