gaborkaszab commented on code in PR #16149:
URL: https://github.com/apache/iceberg/pull/16149#discussion_r3166552846
##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -318,53 +317,69 @@ protected <T extends RESTResponse> T execute(
}
HttpContext context = HttpClientContext.create();
- try (CloseableHttpResponse response = httpClient.execute(request,
context)) {
- Map<String, String> respHeaders = Maps.newHashMap();
- for (Header header : response.getHeaders()) {
- respHeaders.put(header.getName(), header.getValue());
- }
-
- responseHeaders.accept(respHeaders);
-
- // Skip parsing the response stream for any successful request not
expecting a response body
- if (emptyBody(response, responseType)) {
- if (response.getCode() == HttpStatus.SC_NOT_MODIFIED
- && !req.headers().contains(HttpHeaders.IF_NONE_MATCH)) {
- // 304-NOT_MODIFIED is used for freshness-aware loading and requires
an ETag sent to the
- // server via IF_NONE_MATCH header in the request. If no ETag was
sent, we shouldn't
- // receive a 304.
- throw new RESTException(
- "Invalid (NOT_MODIFIED) response for request: method=%s,
path=%s",
- req.method(), req.path());
- }
+ try {
+ return httpClient.execute(
+ request,
+ context,
+ response ->
Review Comment:
Thanks you for the changes @ebyhr !
Here, probably it doesn't matter much, but what I had in mind is to have a
member of the class that implements `HttpClientResponseHandler`, and instead of
a lambda we could pass that member here.
It would be helpful later on to swap that out for some custom handler (maybe
wrap it into one that could verify content of HTTP response) for testing.
Let me know if this is over engineering, or too unrelated to this PR. I
figured since we touch this part, would also make sense.
--
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]
