pkuwm commented on a change in pull request #1472:
URL: https://github.com/apache/helix/pull/1472#discussion_r506746581
##########
File path:
helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientImpl.java
##########
@@ -116,25 +116,36 @@ protected JsonNode getJsonObject(HttpResponse
httpResponse) throws IOException {
LOG.info("Expected HttpResponse statusCode: {}", HttpStatus.SC_OK);
return jsonConverter.convert(getJsonObject(httpResponse));
} else {
+ // Ensure entity is fully consumed so stream is closed.
+ EntityUtils.consumeQuietly(httpResponse.getEntity());
throw new ClientProtocolException("Unexpected response status: " +
status + ", reason: "
+ httpResponse.getStatusLine().getReasonPhrase());
}
}
@VisibleForTesting
protected HttpResponse post(String url, Map<String, String> payloads) throws
IOException {
+ HttpPost postRequest = new HttpPost(url);
try {
- HttpPost postRequest = new HttpPost(url);
postRequest.setHeader("Accept", ACCEPT_CONTENT_TYPE);
StringEntity entity = new
StringEntity(OBJECT_MAPPER.writeValueAsString(payloads),
ContentType.APPLICATION_JSON);
postRequest.setEntity(entity);
LOG.info("Executing request: {}, headers: {}, entity: {}",
postRequest.getRequestLine(),
postRequest.getAllHeaders(), postRequest.getEntity());
- return _httpClient.execute(postRequest);
+
+ HttpResponse response = _httpClient.execute(postRequest);
Review comment:
I tried that. If we move to `handleResponse`, we'll have to change more
code, eg. method signature of `handleResponse` to have an extra parameter
`payloads`, and we need to change the callers as well. That sounds more work to
me and it doesn't look better than this. I prefer this way so it is cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]