-
Notifications
You must be signed in to change notification settings - Fork 3k
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
{Packaging} Deprecation notice urllib3[secure] #23494
Conversation
updated with urllib3
Thank you for your contribution umarfarouk98! We will review the pull request and get back to you soon. |
appservice |
@panchagnula Could you please help review this PR? |
Sorry, I only wanted to delete my own comment, but I deleted the whole comment by mistake |
@StrawnSC could you also check if we don't use any of these libraries in ACA extension & might need to upgrade as well. I can do the testing for this PR/ webapp |
@panchagnula I just checked and we don't use this library in the ACA extension |
Bumping this to bring it back to attention. @zhoxing-ms any chance we can pull this in (assuming a subsequent merge/CI test passes)? Asking because we have some code over in Azure ML that uses the CLI and is seeing deprecation warnings here. Thanks! |
if not getattr(ssl, "HAS_SNI", False): | ||
try: | ||
from urllib3.contrib import pyopenssl | ||
pyopenssl.inject_into_urllib3() |
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.
This code was introduced by #907 in 2016. According to urllib3/urllib3#2680
If you're using Python 2.7.10 or later and OpenSSL 1.1.1+ (this is almost everyone) then you won't need the urllib3[secure] extra or urllib3.contrib.pyopenssl module. Both were necessary a long time ago, but aren't necessary any more!
this line is no longer needed since we have dropped support for Python 2 (#11363) and most distributions now have OpenSSL 1.1.1+ or even 3.0.
In addition, instead of using urllib3
, we should use requests
as unified way to make HTTP requests (#26008).
urllib3
[secure]
extra
urllib3
[secure]
extra
/azp run |
Ok will do so shortly |
@microsoft-github-policy-service agree |
@fyunusa, Thanks for the PR. However, as there are many linter errors:
Could you rebase to the latest I will then deal with |
@@ -149,7 +149,7 @@ | |||
'semver==2.13.0', | |||
'six>=1.10.0', # six is still used by countless extensions | |||
'sshtunnel~=0.1.4', | |||
'urllib3[secure]', | |||
'urllib3', |
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.
urllib3[secure]
installs below packages:
secure = [
"pyOpenSSL>=17.1.0",
"cryptography>=1.9",
"idna>=2.0.0",
"certifi",
"urllib3-secure-extra",
]
We do explicitly require them in our requirements.txt
: https://github.com/Azure/azure-cli/blob/424374558bedf9031069e5a70e712dbe79a2442c/src/azure-cli/requirements.py3.Linux.txt
This complies with the migration guidance: urllib3/urllib3#2700
- If needed, add
pyOpenSSL>=0.14
,cryptography>=1.3.4
,idna>=2
, andcertifi
to their dependencies. These dependencies should only be added back if they're actually used within the project. If they're not used (includingpyopenssl.inject_into_urllib3
) then they can likely be omitted.
Besides, according to https://urllib3.readthedocs.io/en/stable/reference/contrib/pyopenssl.html
However, pyOpenSSL depends on cryptography, which depends on idna, so while we use all three directly here we end up having relatively few packages required.
Ok then I'll work on that |
@jiasli I've sync my repo to get latest update from the dev branch and then re sync the pull request, does that work fine? |
@fyunusa, could you please make the change to let the CI pass? Thanks a lot. |
Ok, will work on it as soon |
You can check now, so i've updated the tunnel.py & custom.py file to match the version on the dev |
@jiasli i think all checks passed now, so you can review the PR and let me know if my attention is needed |
Thank you very much @fyunusa. There is nothing pending on your side now. App Service team will review this PR soon. I will merge it after they approve. |
Description
pyOpenSSL and urllib3[secure] are deprecated in the upcoming release (1.26.12)
urllib3/urllib3#2680