ebyhr commented on code in PR #16149:
URL: https://github.com/apache/iceberg/pull/16149#discussion_r3167337396
##########
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:
@gaborkaszab I don't think we need to address this right now. We can expose
it for testing later if a clear need arises. I extracted the class locally but
discarded it since it didn’t simplify the current code.
--
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]