kevinjqliu commented on code in PR #2846:
URL: https://github.com/apache/iceberg-python/pull/2846#discussion_r2646150788
##########
pyiceberg/io/fsspec.py:
##########
@@ -121,9 +121,19 @@ def __call__(self, request: "AWSRequest", **_: Any) ->
None:
signer_url = self.properties.get(S3_SIGNER_URI,
self.properties[URI]).rstrip("/") # type: ignore
signer_endpoint = self.properties.get(S3_SIGNER_ENDPOINT,
S3_SIGNER_ENDPOINT_DEFAULT)
- signer_headers = {}
+ signer_headers: dict[str, str] = {}
+
+ auth_header: str | None = None
if token := self.properties.get(TOKEN):
- signer_headers = {"Authorization": f"Bearer {token}"}
+ auth_header = f"Bearer {token}"
Review Comment:
yea great point. i think it would be better to split out the "rest signer"
from fileio. there's a good example already in the REST catalog,
https://github.com/apache/iceberg-python/blob/a99dcada431a06329959cb7f71bf961343256bb0/pyiceberg/catalog/rest/__init__.py#L409-L459
it might also be easier to just pass in the request Session from the REST
catalog to the Signer. So we dont need to recreate the auth header directly
but again, we can refactor this after the bug fix :)
--
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]
