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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify packages using urllib3[secure] about it's deprecation #2700

Closed
18 tasks done
sethmlarson opened this issue Aug 4, 2022 · 36 comments
Closed
18 tasks done

Notify packages using urllib3[secure] about it's deprecation #2700

sethmlarson opened this issue Aug 4, 2022 · 36 comments
Labels
馃挵 Bounty $300 If you complete this issue we'll pay you $300 on OpenCollective!
Milestone

Comments

@sethmlarson
Copy link
Member

sethmlarson commented Aug 4, 2022

馃挼 You can get paid to complete this issue! Please read the docs for more information.

Packages to notify

  • selenium
  • scout-apm
  • cpp-coveralls
  • sentry
  • awscurl
  • mastercard-api-core
  • mastercard-oauth1-signer
  • cloud-files
  • google-assistant-sdk
  • minet
  • cloud-volume
  • azure-cli-appservice
  • runway-python
  • lyricsmaster
  • pyramid-oereb
  • flexmeasures
  • optimove
  • comet-git-pure

What to do for each package

  • Find the source repository (GitHub/GitLab/BitBucket/etc)
  • Create a PR which includes the following:
    • Mentions that pyOpenSSL and urllib3[secure] are deprecated in the upcoming release (1.26.12)

    • Links to Deprecate [secure] extra聽#2680

    • Removes [secure] extra from the urllib3 dependencies

    • If needed, add pyOpenSSL>=0.14, cryptography>=1.3.4, idna>=2, and certifi to their dependencies. These dependencies should only be added back if they're actually used within the project. If they're not used (including pyopenssl.inject_into_urllib3) then they can likely be omitted.

    • If urllib3.contrib.pyopenssl.inject_into_urllib3 is used anywhere in the project (search for inject_into_urllib3) then make the change to the following:

      try:
          import ssl
      except ImportError:
          ssl = None
      
      if not getattr(ssl, "HAS_SNI", False):
          from urllib3.contrib import pyopenssl
      
          pyopenssl.inject_into_urllib3()

      This is the logic that is used in Requests to only use pyOpenSSL if SNI isn't available (which is very rare).

  • After creating each PR, add it as a comment to this issue.

PRs don't need to be merged to complete this issue, we only want to create the PRs so that maintainers of these projects are aware of the upcoming changes.

Part of #2680

@sethmlarson sethmlarson added the 馃挵 Bounty $300 If you complete this issue we'll pay you $300 on OpenCollective! label Aug 9, 2022
@sethmlarson sethmlarson added this to the v2.0 milestone Aug 9, 2022
@fyunusa
Copy link

fyunusa commented Aug 10, 2022

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to Selenium
SeleniumHQ/selenium#10932

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to cpp-coveralls
eddyxu/cpp-coveralls#166

@pquentin
Copy link
Member

@umarfarouk98 Make sure to only remove [secure], not the whole urllib3 dependency!

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Yes i only removed secure and other modules aren't tampered with

@pquentin
Copy link
Member

pquentin commented Aug 10, 2022

The current diff for cpp-coveralls looks like this:

  idna>=2.5,<3
  requests>=2.25.1
- urllib3[secure]
  future

This means urllib3 will not be installed at all. Instead it should be like this:

  idna>=2.5,<3
  requests>=2.25.1
- urllib3[secure]
+ urllib3
  future

@sethmlarson
Copy link
Member Author

sethmlarson commented Aug 10, 2022

@umarfarouk98 When you're making changes you should start with urllib3[secure] and should only remove the [secure] part, meaning urllib3 and any version information will still be there.

Example: urllib3[secure]>=1.26, <2 should get changed to urllib3>=1.26, <2

Also if urllib3 1.25.x or earlier is being installed you can leave the [secure] for those cases like scout_apm.

Can you fix your earlier PRs?

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

noted, currently on it

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to mastercard-oauth1-signer
Mastercard/oauth1-signer-python#41

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to cloud-files
seung-lab/cloud-files#77

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to minet
medialab/minet#446

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to cloud-volume
seung-lab/cloud-volume#552

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Hi @sethmlarson the runway-python repo is in archive already.
Screenshot 2022-08-10 at 2 14 25 PM

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to azure-cli
Azure/azure-cli#23494

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created a Pull Request to lyricsmaster
SekouD/lyricsmaster#1094

@sethmlarson
Copy link
Member Author

@umarfarouk98 FYI, you need to also be checking for inject_into_urllib3 and do the other parts of this task if that's present, not only removing [secure]. For example in azure-cli inject_into_urllib3 is present:

https://github.com/Azure/azure-cli/search?q=inject_into_urllib3

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

ok i'll work on that now @sethmlarson

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

modified inject_into_urllib3()
tunnel.py and custom.py azure-cli

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created Pull Request to pyramid-oereb
openoereb/pyramid_oereb#1598

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created Pull Request to flexmeasures
FlexMeasures/flexmeasures#473

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created Pull Request to optimove
nicolasramy/optimove#19
No "inject_into_urllib3()" used

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created Pull Request to awscurl
okigan/awscurl#145
No "inject_into_urllib3()" used

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Hi @sethmlarson
sentry doesn't seem to use the [secure] extra for urllib3
https://github.com/getsentry/sentry-python/

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created Pull Request to google-assistant-sdk
No "inject_into_urllib3()" used
googlesamples/assistant-sdk-python#441

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Created Pull Request to mastercard-api-core
mfuery/mastercard-core-api-python#2
No "inject_into_urllib3()" used

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Hi @sethmlarson please could you point me to the comet-git-pure repo.
finding it hard to locate

@sethmlarson
Copy link
Member Author

@umarfarouk98 If you can't locate the repo that's fine, I also can't find it so we tried our best :)

For the PRs that require a CLA like azure-cli and the Google Assistant one can you sign the CLA so the PR can be accepted? Thanks!

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

@sethmlarson think this might be it, not sure but i created a PR to it
comet-ml/dulwich#1

@fyunusa
Copy link

fyunusa commented Aug 10, 2022

Also i've signed the CLA for both azure and the google assistant-sdk
check to see the PR for google-assistant
googlesamples/assistant-sdk-python#441

@pquentin
Copy link
Member

sentry doesn't seem to use the [secure] extra for urllib3 https://github.com/getsentry/sentry-python/

https://pypi.org/project/sentry/ on PyPI is the Sentry server, whose repo is https://github.com/getsentry/sentry. They don't require [secure] in their direct dependencies, but they depend on Selenium. So there's nothing to do yet.

@sethmlarson think this might be it, not sure but i created a PR to it comet-ml/dulwich#1

This is only a fork, I think upstream is https://github.com/jelmer/dulwich. It has active pull requests. Can you please open a pull request there instead?

@fyunusa
Copy link

fyunusa commented Aug 11, 2022

ok will create the pull request to https://github.com/jelmer/dulwich. now

@fyunusa
Copy link

fyunusa commented Aug 11, 2022

created pull request to jelmer/dulwich
jelmer/dulwich#998
No "inject_into_urllib3()" used

@fyunusa
Copy link

fyunusa commented Aug 11, 2022

Hi @sethmlarson @pquentin is there anything left for me to do ?

@sethmlarson
Copy link
Member Author

@umarfarouk98 Let me run one more query to make sure we've got all the packages in the list. I'll let you know by the end of the day if there's any more work to do.

Thanks for all your work so far!

@sethmlarson
Copy link
Member Author

@umarfarouk98 Okay I checked the list again and it looks like every package is covered, thank you for doing this. You can submit the invoice to the Open Collective for $300.

I just ask that you don't close the PRs you've opened so the maintainers of those repositories can see them and if you have any time to get them across the finish line it'd be appreciated but not necessary to be paid for the work you've already done. Thanks much!

@fyunusa
Copy link

fyunusa commented Aug 12, 2022

OK @sethmlarson thanks to you too, I'll try to do a follow up on the PRs also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
馃挵 Bounty $300 If you complete this issue we'll pay you $300 on OpenCollective!
Projects
None yet
Development

No branches or pull requests

3 participants