Skip to content

use getResponseBodyAsStream for potential large response#42

Closed
fengxx wants to merge 1 commit intojenkinsci:masterfrom
fengxx:feature/stream_response
Closed

use getResponseBodyAsStream for potential large response#42
fengxx wants to merge 1 commit intojenkinsci:masterfrom
fengxx:feature/stream_response

Conversation

@fengxx
Copy link

@fengxx fengxx commented Apr 7, 2017

Got lots of warning

Going to buffer response body of large or unknown size.
Using getResponseBodyAsStream instead is recommended.

It seems that bitbucket server didn't include contentLength in response, although it is ok to ignore the warning, I think rewrite getRequest & parse using stream api may benefit readability and performance.

Note: BitbucketCloudApiClient is kept intact because too much hack in deserialization .

private <T> T getResponse(String path, Class<T> clazz) throws IOException {
GetMethod httpget = new GetMethod(this.baseURL + path);
HttpClient client = getHttpClient(getMethodHost(httpget));
try {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend to use the CloseableResponse with try-with-resources (http://stackoverflow.com/questions/21574478/what-is-the-difference-between-closeablehttpclient-and-httpclient-in-apache-http), that way you correctly release the connection even in case of errors

input.close();
return object;
} catch (BitbucketRequestException | FileNotFoundException e) {
LOGGER.log(Level.SEVERE, "Request failed for url:" + path, e);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either log or throw but not both

LOGGER.log(Level.SEVERE, "Request failed for url:" + path, e);
throw e;
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Communication error for url: " + path, e);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either log or throw but not both

@stephenc
Copy link
Member

superseded by #53

@stephenc stephenc closed this Jun 12, 2017
fengxx pushed a commit to fengxx/bitbucket-branch-source-plugin that referenced this pull request Mar 13, 2018
Switched from SCMTrigger.SCMTriggerCause to BranchIndexingCause
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants