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

Fix setting verify for requests #219

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

rbikar
Copy link
Member

@rbikar rbikar commented Oct 29, 2023

Due to bug psf/requests#3829, setting 'verify' on session doesn't work if REQUESTS_CA_BUNDLE is set on environment. The bundle defined via REQUESTS_CA_BUNDLE will take precedence and the custom bundle provided via 'verify' is ignored.

Let's now set 'verify' for each request.

This can be revetrted when the bug is fixed, likely in python-requests-v3.

Due to bug psf/requests#3829,
setting 'verify' on session doesn't work if REQUESTS_CA_BUNDLE
is set on environment. The bundle defined via REQUESTS_CA_BUNDLE
will take precedence and the custom bundle provided via 'verify'
is ignored.

Let's now set 'verify' for each request.

This can be revetrted when the bug is fixed, likely in
python-requests-v3.
@codecov
Copy link

codecov bot commented Oct 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c7c2bd0) 96.89% compared to head (d0d0535) 96.89%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #219   +/-   ##
=======================================
  Coverage   96.89%   96.89%           
=======================================
  Files           8        8           
  Lines         997      998    +1     
=======================================
+ Hits          966      967    +1     
  Misses         31       31           
Files Coverage Δ
ubipop/_cdn.py 98.75% <100.00%> (+<0.01%) ⬆️

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

@rbikar rbikar marked this pull request as ready for review October 29, 2023 17:27
@rbikar rbikar requested review from jm-wk and rohanpm as code owners October 29, 2023 17:27
# set verify for each request
# verify set on session doesn't work due to https://github.com/psf/requests/issues/3829
# if REQUESTS_CA_BUNDLE is set on env, it takes precedence
kwargs["verify"] = self._session.verify
Copy link
Member

Choose a reason for hiding this comment

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

I'm not certain which is better, but there is also a trust_env var you can set on the session to False.

Lately my approach has been: if the Session will have either verify or proxies used, then always set explicitly (verify, proxies, trust_env=False).

Copy link
Member Author

@rbikar rbikar Oct 30, 2023

Choose a reason for hiding this comment

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

your approach sounds also good, I guess I'll keep it simple for the time being.

@rbikar rbikar merged commit 5f07b2c into release-engineering:master Oct 30, 2023
7 checks passed
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.

2 participants