requests / toolbelt

A toolbelt of useful classes and functions to be used with python-requests
https://toolbelt.readthedocs.org
Other
989 stars 186 forks source link

Urllib3 Removed AppEngine Support in v2.0.0 #354

Closed crsnchng closed 1 year ago

crsnchng commented 1 year ago

Apologies if this is structured poorly, I don't open issues often. I use Zeep to access a SOAP API via Python and they import MultipartDecoder to use here. Since the release of Urllib3 v2.0.0 yesterday, I've been getting errors and I was finally able to figure out what happened (I feel dumb, I should have just looked at the call stack more closely).

Release for reference: https://github.com/urllib3/urllib3/releases/tag/2.0.0

Removed urllib3.contrib.appengine.AppEngineManager and support for Google App Engine Standard Environment (https://github.com/urllib3/urllib3/issues/2044).

Where I was able to locate the reference to AppEngine in toolbelt:

https://github.com/requests/toolbelt/blob/9d47ccf1a2e0f033f9b7d9c269e455c7f505f0fd/requests_toolbelt/_compat.py#L44-L50

The error call stack from my code that led me here:

from requests_toolbelt.multipart.decoder import MultipartDecoder File "/home/site/wwwroot/.python_packages/lib/site-packages/requests_toolbelt/init.py", line 12, in module from .adapters import SSLAdapter, SourceAddressAdapter File "/home/site/wwwroot/.python_packages/lib/site-packages/requests_toolbelt/adapters/init.py", line 12, in module from .ssl import SSLAdapter File "/home/site/wwwroot/.python_packages/lib/site-packages/requests_toolbelt/adapters/ssl.py", line 16, in module from .._compat import poolmanager File "/home/site/wwwroot/.python_packages/lib/site-packages/requests_toolbelt/_compat.py", line 50, in module from urllib3.contrib import appengine as gaecontrib

sigmavirus24 commented 1 year ago

@crsnchng This is plenty good enough for a bug report. You have all the right information.

For what it's worth zeep seems to not require urllib3 directly (in setup.py). What that tells me is that something else you're relying on isn't capping urllib3 and so you're ending up with 2.0. If you have it in your dependencies or can track that dependency down, you can probably cap urllib3 for the time being to get out of the immediate pain.

We should probably make sure that whatever is relying on this support is killed in a major release. Until then, if your other dependency(ies?) requiring urllib3 aren't requiring 2.0 I think this can keep you out of this particular problem

If you'd like to send a PR to rip out the appengine/gae code, that would be greatly appreciated and I would happily review it.


@pquentin alternatively, do you have time to get to this now that 2.0 is out?

crsnchng commented 1 year ago

@sigmavirus24 Thank you for the reply.

Before I wrote the initial issue up, I forced the Urllib3 version back to the previous one and everything worked again, so I'm good for now.

I would absolutely be interested in helping with that however I don't know that I have the ability to do this accurately. I wouldn't know where to begin aside from searching for appengine in the code and removing it and the surrounding context from everywhere I see it.

sigmavirus24 commented 1 year ago

It's likely the import in the middle you got the trade back from and wherever we import those values to (likely the whole file) if I remember correctly. I expect it to be relatively straight forward

pquentin commented 1 year ago

Thanks!

@sigmavirus24 To be clear you would be OK with removing App Engine support and generally make sure that toolbelt is compatible with urllib3 2.0 and all its removals/deprecations and then release that as a new version?

I can help with that.

sigmavirus24 commented 1 year ago

Yes that's what I'm in favor of. I don't think the app engine support has been used/needed for years

pquentin commented 1 year ago

requests-toolbelt is out with urllib3 2.0 support: https://pypi.org/project/requests-toolbelt/1.0.0/

Thanks for the report, closing.