Skip to content

Consistent handling of HTTP/S proxy environment variables#2079

Open
davidwegs wants to merge 2 commits intoapache:trunkfrom
davidwegs:no_proxy_support
Open

Consistent handling of HTTP/S proxy environment variables#2079
davidwegs wants to merge 2 commits intoapache:trunkfrom
davidwegs:no_proxy_support

Conversation

@davidwegs
Copy link

Consistent handling of HTTP/S proxy environment variables

Description

This PR provides consistent handling of the http_proxy, https_proxy, and no_proxy environment variables in line with the behavior of the Requests python library. Previously no_proxy wasn't being respected (#2077).

This is done by removing explicit handling of the http_proxy and https_proxy env variables in favor of downstream handling through the Requests library. Explicitly provided proxy settings through the proxy_url argument to the connection class and through the set_http_proxy method are still supported and still take precedence over the environment variables.

Status

done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

Comment on lines -107 to -116
proxy_url = "http://127.0.0.2:3128"
os.environ["http_proxy"] = proxy_url
conn = LibcloudConnection(host="localhost", port=80)
self.assertEqual(conn.proxy_scheme, "http")
self.assertEqual(conn.proxy_host, "127.0.0.2")
self.assertEqual(conn.proxy_port, 3128)
self.assertEqual(
conn.session.proxies,
{"http": "http://127.0.0.2:3128", "https": "http://127.0.0.2:3128"},
)
Copy link
Member

Choose a reason for hiding this comment

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

Can we verify the underneath library still hold the proxy settings?

Copy link
Author

Choose a reason for hiding this comment

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

Just added a separate unit test for this. I can't verify it in this one because the Requests library checks the proxy env variables when processing a request not when creating a Session.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.36%. Comparing base (c899034) to head (a521a61).
⚠️ Report is 125 commits behind head on trunk.

Files with missing lines Patch % Lines
libcloud/test/test_connection.py 93.75% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2079      +/-   ##
==========================================
- Coverage   83.39%   83.36%   -0.03%     
==========================================
  Files         353      353              
  Lines       81696    81906     +210     
  Branches     8634     9007     +373     
==========================================
+ Hits        68123    68276     +153     
- Misses      10750    10800      +50     
- Partials     2823     2830       +7     
Files with missing lines Coverage Δ
libcloud/http.py 92.09% <100.00%> (-0.22%) ⬇️
libcloud/test/test_connection.py 97.64% <93.75%> (+0.14%) ⬆️

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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