gaborkaszab commented on code in PR #16149:
URL: https://github.com/apache/iceberg/pull/16149#discussion_r3167457483
##########
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:
I didn't mean to expose it now, it would be out of scope for this PR. What I
meant is that instead having a `handleResponse` function and using it in the
lambda, we could have an `HttpClientResponseHandler` implementation as a member
of this class.
No strong feelings though, up to you.
--
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]
