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

Allow keep-alive with proxy for message with appropriate header #5764

Closed
wants to merge 3 commits into from

Conversation

avderin
Copy link

@avderin avderin commented Jun 4, 2021

What do these changes do?

Once ago there was a merged pr - #3070 that disabled keep-alive connections through the proxy. There was a suggestion to make this functionality switchable in discussion.

Keep-alive with the proxy sometimes is need. E.g. to establish ntlm authorization the client must send 2 requests in one connection (a handshake). Current aiohttp logic restricts it, closing the connection on the first request.

I suggest to allow keep-alive connection when client explicitly sets keep-alive header.

Related issue number

#5765

@avderin avderin requested a review from asvetlov as a code owner June 4, 2021 15:17
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jun 4, 2021
@webknjaz
Copy link
Member

A change like this would require regression tests.

Copy link

codecov bot commented Aug 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.24%. Comparing base (1d21dce) to head (5118422).
Report is 1048 commits behind head on master.

Files with missing lines Patch % Lines
aiohttp/connector.py 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5764      +/-   ##
==========================================
- Coverage   98.25%   98.24%   -0.01%     
==========================================
  Files         107      107              
  Lines       34125    34126       +1     
  Branches     4048     4049       +1     
==========================================
- Hits        33530    33528       -2     
- Misses        421      423       +2     
- Partials      174      175       +1     
Flag Coverage Δ
CI-GHA 98.14% <0.00%> (-0.01%) ⬇️
OS-Linux 97.80% <0.00%> (-0.01%) ⬇️
OS-Windows 96.20% <0.00%> (-0.01%) ⬇️
OS-macOS 97.47% <0.00%> (-0.01%) ⬇️
Py-3.10.11 97.57% <0.00%> (-0.01%) ⬇️
Py-3.10.14 97.50% <0.00%> (-0.01%) ⬇️
Py-3.11.9 97.73% <0.00%> (-0.01%) ⬇️
Py-3.12.4 96.00% <0.00%> (-0.01%) ⬇️
Py-3.12.5 97.53% <0.00%> (-0.01%) ⬇️
Py-3.9.13 97.46% <0.00%> (-0.01%) ⬇️
Py-3.9.19 97.40% <0.00%> (-0.01%) ⬇️
Py-pypy7.3.16 97.01% <0.00%> (-0.01%) ⬇️
VM-macos 97.47% <0.00%> (-0.01%) ⬇️
VM-ubuntu 97.80% <0.00%> (-0.01%) ⬇️
VM-windows 96.20% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dreamsorcerer
Copy link
Member

Superseded by #8920.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants