amogh-jahagirdar commented on code in PR #15648:
URL: https://github.com/apache/iceberg/pull/15648#discussion_r2941383572
##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -340,32 +339,23 @@ protected <T extends RESTResponse> T execute(
return null;
}
- String responseBody = extractResponseBodyAsString(response);
-
if (!isSuccessful(response)) {
// The provided error handler is expected to throw, but a
RESTException is thrown if not.
+ String responseBody = extractResponseBodyAsString(response);
throwFailure(response, responseBody, errorHandler);
}
- if (responseBody == null) {
+ if (response.getEntity() == null) {
throw new RESTException(
"Invalid (null) response body for request (expected %s):
method=%s, path=%s, status=%d",
responseType.getSimpleName(), req.method(), req.path(),
response.getCode());
}
- try {
- ObjectReader reader = objectReaderCache.computeIfAbsent(responseType,
mapper::readerFor);
- if (parserContext != null && !parserContext.isEmpty()) {
- reader = reader.with(parserContext.toInjectableValues());
- }
- return reader.readValue(responseBody);
- } catch (JsonProcessingException e) {
- throw new RESTException(
- e,
- "Received a success response code of %d, but failed to parse
response body into %s",
- response.getCode(),
- responseType.getSimpleName());
+ ObjectReader reader = objectReaderCache.computeIfAbsent(responseType,
mapper::readerFor);
+ if (parserContext != null && !parserContext.isEmpty()) {
+ reader = reader.with(parserContext.toInjectableValues());
}
+ return reader.readValue(response.getEntity().getContent());
Review Comment:
Looks like Jackson takes the responsibility here of closing the input stream
internally (which is fine and expected). Just wanted to make sure this doesn't
break any assumptions at the HTTPClient level around connection reuse but don't
think it does , so I think this solution is strictly better!
--
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]