Support websocket proxies configured via environment variables by mmasztalerczuk · Pull Request #4661 · aio-libs/aiohttp

mmasztalerczuk · 2020-03-24T20:00:31Z

What do these changes do?

Fixes a problem with the non-working configuration of the proxy
for wss (when a proxy URL was set via env var).

Are there changes in behavior for the user?

Working proxy

Related issue number

Resolves #4648

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

PR aio-libs#4648

webknjaz

I have some concerns about this change.

Co-Authored-By: Sviatoslav Sydorenko <[email protected]>

mmasztalerczuk · 2020-03-25T17:09:07Z

Thanks for review :)

Co-Authored-By: Sviatoslav Sydorenko <[email protected]>

codecov-io · 2020-03-25T23:27:35Z

Codecov Report

Merging #4661 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4661      +/-   ##
==========================================
+ Coverage   97.58%   97.59%   +0.01%     
==========================================
  Files          43       43              
  Lines        8916     8916              
  Branches     1405     1405              
==========================================
+ Hits         8701     8702       +1     
  Misses         95       95              
+ Partials      120      119       -1

Impacted Files	Coverage Δ
aiohttp/helpers.py	`96.61% <100.00%> (ø)`
aiohttp/web_fileresponse.py	`97.82% <0.00%> (-0.55%)`	⬇️
aiohttp/streams.py	`97.92% <0.00%> (+0.51%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79f47ab...097acf4. Read the comment docs.

PR #4661 Resolves #4648 Co-authored-by: Sviatoslav Sydorenko <[email protected]>

PR aio-libs#4661 Resolves aio-libs#4648 Co-authored-by: Sviatoslav Sydorenko <[email protected]>

patchback · 2021-10-31T18:34:16Z

Backport to 3.8: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 50753ea on top of patchback/backports/3.8/50753eace15da24a9fb9774bcfcaa1dc7650164e/pr-4661

Backporting merged PR #4661 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.8/50753eace15da24a9fb9774bcfcaa1dc7650164e/pr-4661 upstream/3.8

Now, cherry-pick PR Support websocket proxies configured via environment variables #4661 contents into that branch:
```
$ git cherry-pick -x 50753eace15da24a9fb9774bcfcaa1dc7650164e
```
If it'll yell at you with something like fatal: Commit 50753eace15da24a9fb9774bcfcaa1dc7650164e is a merge but no -m option was given., add -m 1 as follows intead:
```
$ git cherry-pick -m1 -x 50753eace15da24a9fb9774bcfcaa1dc7650164e
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Support websocket proxies configured via environment variables #4661 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.8/50753eace15da24a9fb9774bcfcaa1dc7650164e/pr-4661

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

PR aio-libs#4661 Resolves aio-libs#4648 Co-authored-by: Sviatoslav Sydorenko <[email protected]> (cherry picked from commit 50753ea)

…d via environment variables (#6183) Co-authored-by: Sviatoslav Sydorenko <[email protected]> Co-authored-by: Mariusz Masztalerczuk <[email protected]>

ws_connect doesn't follow proxy from environment

03a7a20

PR aio-libs#4648

mmasztalerczuk requested review from asvetlov and webknjaz as code owners March 24, 2020 20:00

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Mar 24, 2020

webknjaz requested changes Mar 25, 2020

View reviewed changes

Comment thread aiohttp/helpers.py Outdated

Comment thread tests/test_helpers.py Outdated

Comment thread tests/test_helpers.py Outdated

Comment thread tests/test_helpers.py Outdated

mmasztalerczuk and others added 3 commits March 25, 2020 17:15

Update aiohttp/helpers.py

f4b9d7d

Co-Authored-By: Sviatoslav Sydorenko <[email protected]>

Update tests/test_helpers.py

d1e3de9

Co-Authored-By: Sviatoslav Sydorenko <[email protected]>

Review fixes

80de7ea