JingsongLi commented on code in PR #8219:
URL: https://github.com/apache/paimon/pull/8219#discussion_r3417540350
##########
paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java:
##########
@@ -86,9 +88,65 @@ private static HttpClientConnectionManager
configureConnectionManager() {
public static InputStream getAsInputStream(String uri) throws IOException {
HttpGet httpGet = new HttpGet(uri);
CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpGet);
- if (response.getCode() != 200) {
- throw new RuntimeException("HTTP error code: " +
response.getCode());
+ int statusCode = response.getCode();
+ if (statusCode != HttpStatus.SC_OK) {
+ throw httpError(statusCode);
Review Comment:
This path leaks the HTTP response on non-200 statuses. `getAsInputStream`
now turns a 404 into the signal that `BlobFormatWriter` catches for
`blob-write-null-on-missing-file`, so repeated missing HTTP blobs can leave
`CloseableHttpResponse`s open until the connection pool is exhausted. Could we
close the response before throwing (for example with try-with-resources on the
error branch, while still returning a stream that owns the response on the 200
branch)? A test that repeatedly opens missing URLs would catch this.
--
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]
