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

Urllib3 2.0 released yesterday breaks latest release due to requirements.txt not limiting urllib3 to use use version less than 2.0 #201

Closed
prpnmac opened this issue Apr 27, 2023 · 13 comments

Comments

@prpnmac
Copy link
Contributor

prpnmac commented Apr 27, 2023

Is this a support request?
This issue tracker is maintained by LaunchDarkly SDK developers and is intended for feedback on the SDK code. If you're not sure whether the problem you are having is specifically related to the SDK, or to the LaunchDarkly service overall, it may be more appropriate to contact the LaunchDarkly support team; they can help to investigate the problem and will consult the SDK team if necessary. You can submit a support request by going here or by emailing support@launchdarkly.com.

Note that issues filed on this issue tracker are publicly accessible. Do not provide any private account information on your issues. If your problem is specific to your account, you should submit a support request as described above.

Describe the bug

The requirements.txt file defines urllib3 as more than 1.22.0 and does not limit it to only use urllib3 v1.x.x.
Urlib3 v2.0 came out yesterday https://github.com/urllib3/urllib3/releases and contains breaking changes to your library. https://urllib3.readthedocs.io/en/stable/v2-migration-guide.html

To reproduce
Install latest version of your library

Expected behavior
Install compatible verison of urllib3. No more than urllib3 1.26.15 https://github.com/urllib3/urllib3/releases/tag/1.26.15

Logs
None I can show

SDK version
Latest but reproducible in all recent released

Language version, developer tools
Python3

OS/platform
Linux/AWS

Additional context

Solution

In your requiremens.txt file please define urllib3 dependency as:

urllib3>=1.22.0,<1.27

@prpnmac
Copy link
Contributor Author

prpnmac commented Apr 27, 2023

PR: #202

@keelerm84
Copy link
Member

Thank you for filing this issue and submitting an initial implementation of a fix for this.

In reviewing the changelog notes for urllib, it seems like this SDK could continue supporting both the 1.x and 2.x branch. Did you see specific errors when using urllib3?

I received a couple of deprecation warnings about the way we are accessing headers, but it seems like we can update that inside of put a version limit on the library itself.

@prpnmac
Copy link
Contributor Author

prpnmac commented Apr 27, 2023

Yes, I noticed the following log:

[ERROR] Runtime.ImportModuleError: Unable to import module 'handler': cannot import name 'DEFAULT_CIPHERS' from 'urllib3.util.ssl_' (/var/task/urllib3/util/ssl_.py)

Which is due to something noted in the migration guide:

Removed the default set of TLS ciphers, instead now urllib3 uses the list of ciphers configured by the system.

Leads me to believe that there is a risk with leaving the install proceed with the latest version of urllib3.

The limitation to not installing version 2.0 or urllib3 is more of a safe approach to make sure systems don't break due to nested dependencies.

I figure it's a reasonable temporary solution until more issues with the new version of urllib3 are known. Not a dig at yall, but making the package a little extra resilient.

LaunchDarklyReleaseBot pushed a commit that referenced this issue May 1, 2023
…ments

(U2C 13) context kind logic for big segments + enable big segment contract tests
@keelerm84
Copy link
Member

Fixed in 8.1.2

Thank you again for your fix!

@prpnmac
Copy link
Contributor Author

prpnmac commented May 1, 2023

Thank you for the prompt resolution 😃

@hauntsaninja
Copy link
Contributor

@prpnmac do you have more of a stacktrace? I'd like to be able to use urllib3 / launchdarkly's use of urllib3 is quite straightforward, so would be good to understand exactly what the issue you saw was

@prpnmac
Copy link
Contributor Author

prpnmac commented Aug 22, 2023

@hauntsaninja Yes, of course!

Here's the link to the PR: #202

  • urllib3 v2.0 was released with (expected) breaking changes to users of urllib3 < 2.0
  • This package's urllib3 dependency was set as >=1.22.0
  • The PR is just updating the requirements.txt file to do urllib3>=1.22.0,<2.0.0

@hauntsaninja
Copy link
Contributor

hauntsaninja commented Aug 22, 2023

Thanks for the quick reply! I understand the PR, but launchdarkly appears to work for me with urllib3 v2, so I'm trying to understand what the specific problem you ran into is, and whether it is alright for launchdarkly to unpin now. You mentioned you ran into some ImportError, do you have a stacktrace or a reproducer?

@prpnmac
Copy link
Contributor Author

prpnmac commented Aug 22, 2023

Yes, the error we were getting was due to the breaking change around removing the default cyphers. Covered by: urllib3/urllib3#2082

The stacktrace was similar to

Unable to import module 'lambda_function': cannot import name 'DEFAULT_CIPHERS' from 'urllib3.util.ssl_' (/opt/python/urllib3/util/ssl_.py)

@keelerm84
Copy link
Member

@prpnmac I am looking at adding back urllib3 v2 support and I wanted to touch base with you since you originally raised this issue.

In my testing locally, the SDK works fine with urllib3 v2 for normal usage. If you want to make use of dynamodb, the boto dependency is going to force you back down to urllib3 1.x, but nothing in our requirements would prevent you from installing that older version.

I assume you are using boto3 given your lambda function error. If you downgrade to v8.1.1 of our SDK, does pip install urllib3 v2 despite botocore's <1.27 requirement?

@prpnmac
Copy link
Contributor Author

prpnmac commented Oct 2, 2023

@keelerm84 Appreciate the follow up

We adjusted on our end and this will not be an issue.
The path forward for upgrade is clear! :)

@keelerm84
Copy link
Member

urllib3 v2 is now supported in 8.1.7

@prpnmac
Copy link
Contributor Author

prpnmac commented Oct 5, 2023

Excellent, thank you for everything!

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

No branches or pull requests

3 participants