amogh-jahagirdar commented on code in PR #15110:
URL: https://github.com/apache/iceberg/pull/15110#discussion_r2747024752
##########
aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java:
##########
@@ -158,7 +158,10 @@ private RESTClient httpClient() {
// Don't include a base URI because this client may be used for
contacting different
// catalogs.
httpClient =
-
HTTPClient.builder(properties()).withObjectMapper(S3ObjectMapper.mapper()).build();
+ HTTPClient.builder(properties())
+ .withHeaders(RESTUtil.configHeaders(properties()))
Review Comment:
I think this is fine, but have we looked at changing HTTPClient itself so
that when we do a builder(properties), we already set the "base" set of headers
from RESTUtil.configHeaders. And then I'd expect withHeader(s) to just merge in
any overrides. I'd need to check further to see if this would cause any issues
anywhere else so don't want to hold the PR up on that, but my thinking is we
generally by default would want to make sure we're sending along any headers
rather than force people building the client to explicitly be aware of that.
--
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]
