search

pynamodb / PynamoDB

A pythonic interface to Amazon's DynamoDB
http://pynamodb.readthedocs.io
MIT License
2.44k stars 427 forks source link

Vendored Botocore Requests #558

Open mikegrima opened 5 years ago

mikegrima commented 5 years ago

Hello All!

Botocore has begun un-vendoring the Requests library, and replacing it with urllib3: https://github.com/boto/botocore/pull/1495.

PynamoDB is making use of the vendored botocore Requests library here: https://github.com/pynamodb/PynamoDB/blob/master/pynamodb/connection/base.py#L19-L20

As such, this results in breakage when unit testing with the latest version of moto (https://github.com/spulec/moto), which addresses this change.

Is there a major level of effort required to swap out the vendored requests? I would be happy to submit a PR, but I don't have a ton of knowledge on the lower-level logic of PynamoDB. I would imagine that this will need to be done anyway, since botocore is migrating away from making use of the vendored Requests library.

mikegrima commented 5 years ago

I just tried making a custom pynamodb_settings.py file that looks like this:

import requests

session_cls = requests.Session

This is working properly now for my unit tests 👏 . I'm going to submit a PR that swaps out the vendored Requests library with the non-vendored one and we can then test if it all works the same.

mikegrima commented 5 years ago

Submitted a PR that should hopefully address this properly at: #559.

kyleknap commented 5 years ago

I agree with depending directly on requests instead of the vendored version of requests. The fact that we vendor requests is an implementation detail, and it is likely that we remove it in future versions of botocore since we are relying on urllib3 now.

mikegrima commented 5 years ago

@kyleknap Is on the AWS SDK team BTW.

asottile commented 5 years ago

additionally, the vendored copy in botocore is vulnerable to CVE 2018-18074

dmulter commented 5 years ago

Note that I'm getting a failure using the latest moto, but it's not the exact problem described here. The fix suggested above did not work for me. All my tests pass using moto 1.3.6, but the new one results in failures for mocked DynamoDB tests.

An item is added to a table with no failure, but a subsequent read doesn't return the item data. I'm waiting to see if the fix for this resolves my issue or not. If it doesn't, I'll get more detail and maybe make another issue. Not sure if it's a moto or pynamodb problem, but clearly the tests that always worked are now failing in simple ways with moto 1.3.7.

EDIT: Patching in the latest Requests library solved my problem. Still awaiting the permanent fix for this issue.

Jamim commented 5 years ago

Related PR: boto/botocore#1817