Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drain before response.Body.Close() #1772

Merged
merged 1 commit into from
Dec 1, 2021
Merged

Conversation

ZhengzheYang
Copy link
Contributor

According to discussion here, we need to drain before we close the body if EOF is not reached.

Also, from client.Do documentation,

If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close. If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport) may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.

Fixing istio/istio#35677

@istio-policy-bot
Copy link

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 1, 2021
@google-cla google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Dec 1, 2021
@istio-testing
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ZhengzheYang
Copy link
Contributor Author

/test all

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 1, 2021
@ZhengzheYang ZhengzheYang marked this pull request as ready for review December 1, 2021 22:58
@ZhengzheYang ZhengzheYang requested a review from a team as a code owner December 1, 2021 22:58
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 1, 2021
@istio-testing istio-testing merged commit e9c313d into istio:master Dec 1, 2021
Shuanglu pushed a commit to Shuanglu/istio-tools that referenced this pull request Jun 30, 2022
Shuanglu pushed a commit to Shuanglu/istio-tools that referenced this pull request Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants