lwfitzgerald commented on code in PR #2543:
URL: https://github.com/apache/iceberg-python/pull/2543#discussion_r2387196169
##########
pyiceberg/io/fsspec.py:
##########
@@ -95,38 +97,60 @@
from botocore.awsrequest import AWSRequest
-def s3v4_rest_signer(properties: Properties, request: "AWSRequest", **_: Any)
-> "AWSRequest":
- signer_url = properties.get(S3_SIGNER_URI, properties[URI]).rstrip("/") #
type: ignore
- signer_endpoint = properties.get(S3_SIGNER_ENDPOINT,
S3_SIGNER_ENDPOINT_DEFAULT)
+class S3RequestSigner(abc.ABC):
+ """Abstract base class for S3 request signers."""
- signer_headers = {}
- if token := properties.get(TOKEN):
- signer_headers = {"Authorization": f"Bearer {token}"}
- signer_headers.update(get_header_properties(properties))
+ properties: Properties
- signer_body = {
- "method": request.method,
- "region": request.context["client_region"],
- "uri": request.url,
- "headers": {key: [val] for key, val in request.headers.items()},
- }
+ def __init__(self, properties: Properties) -> None:
+ self.properties = properties
+
+ @abc.abstractmethod
+ def __call__(self, request: "AWSRequest", **_: Any) -> None:
+ pass
+
+
+class S3V4RestSigner(S3RequestSigner):
+ """An S3 request signer that uses an external REST signing service to sign
requests."""
+
+ session: requests.Session
- response = requests.post(f"{signer_url}/{signer_endpoint.strip()}",
headers=signer_headers, json=signer_body)
- try:
- response.raise_for_status()
- response_json = response.json()
- except HTTPError as e:
- raise SignError(f"Failed to sign request {response.status_code}:
{signer_body}") from e
+ def __init__(self, properties: Properties) -> None:
+ super().__init__(properties)
+ self.session = requests.Session()
- for key, value in response_json["headers"].items():
- request.headers.add_header(key, ", ".join(value))
+ def __call__(self, request: "AWSRequest", **_: Any) -> None:
Review Comment:
Yes - The place in `botocore` where the `before-sign.s3` event is emitted
expects the passed in `AWSRequest` to be mutated, and doesn't use the return
value -
https://github.com/boto/botocore/blob/dc5ce63e11ad7bea6673caece94030d7be1bb65a/botocore/signers.py#L156-L165
--
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]
