-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[Python][Client] Default to system CA instead of certifi #8108
Conversation
Yes I think that makes sense to better document the I have not touched the I also notice that running |
Thank you for adding that update. asyncio is still built from python-legacy. If you want to build this into python-legacy that's fine too. I have no preference either way. What do you prefer? |
Sure, I will add to python-legacy. It looks like the changes should be the same. |
I have added the same changes to python-legacy Not sure if the CircleCI error is related to my changes. The error seems to be related to an unavailable dependency:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for you PR and adding the new __init__
argument! This looks great!
Resolves #6506
Gets the Python client to use the system Certificate Authority bundle by default for verifying ssl connections (like the Python standard library does, as described in PEP 476) instead of relying on the external certifi python package for the default. See #6506 for the reasons for this change.
For the urllib3 python client this is achieved by passing
configuration.ssl_ca_cert
directly to urllib3.PoolManager or urllib3.ProxyManager’sca_certs
argument. Thenca_certs
beNone
by default and in this case urllib3 (from version 1.25.3) will load the system CA certificates, as described here.I have done the same with the asyncio python client since the
ssl.create_default_context
will also use the system’s default CA certificates ifcafile
,capath
andcadata
are allNone
, as documented here.I have additionally:
certifi
as a dependencyurllib3 >= 1.25.3
These changes would be breaking for anyone relying on the certifi certificates, although I imagine most people’s system default CA certificates are adequate and in this case the change would not be noticed. Any user still wanting to use the certifi certificates could continue to do so by setting
configuration.ssl_ca_cert=certifi.where()
.PR checklist
./bin/generate-samples.sh
to update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example./bin/generate-samples.sh bin/configs/java*
. For Windows users, please run the script in Git BASH.master
@taxpon @frol @mbohlool @cbornet @kenjones-cisco @tomplus @Jyhess @arun-nalla @spacether