Use binary_type in MongoDBStore.__getitem__ for Python 2 only #401
Conversation
|
Hello @jakirkham! Thanks for updating the PR.
Comment last updated on February 13, 2019 at 06:37 Hours UTC |
|
Still seems to be failing. This by the way was why I opened #393. |
On both Python 2 and Python 3, PyMongo converts `binary_type` (i.e. `bytes`) to BSON type 5 (Binary data) with subtype 0 to store in the MongoDB instance. When reading that data back out, PyMongo handles BSON type 5 (Binary data) with subtype 0 differently depending on the Python version. On Python 2, it creates a `bson.Binary` instance with the data. Normally we would coerce this to a `bytes` object using `ensure_bytes`. However that fails as `bson.Binary` is a subclass of `bytes`. So instead we explicitly force it to `bytes` (i.e. `binary_type`). On Python 3, PyMongo automatically converts the data to `bytes`. Thus we don't need to do anything there. ref: http://api.mongodb.com/python/current/python3.html#id3 ref: http://api.mongodb.com/python/current/api/bson/binary.html#bson.binary.Binary
jakirkham
changed the title
|
That's right. Just wanted to see if that was still the case after the dust settled. After doing a little research, it looks like we can safely constrain this coercion to Python 2 only. Have updated the PR to do that. Should make it easier for us to remember to remove when we do drop Python 2. Could you please give it another look? :) |
|
yep, this looks right. I agree, this will make it easier to undo when PY2 goes away. Thanks. |
|
Thanks @jhamman :) |
|
Thanks @jakirkham for this, nice to know exactly what's going on 👍 |
|
Of course. 😄 FWIW I think we can get a better handle on this behavior by adding a flag to the |
Previously there were some test failures on Python 2 and referenced in this comment (and later discussion). Turns out on Python 2 a
bson.Binaryinstance is returned, which subclassesbytes, as explained in the docs. So the coercion tobytesis only needed on Python 2 to convertbson.Binarytobytes. As Python 3 already returns abytesinstance, there is nothing we need to do there. Thus we restrict thebinary_typecall to Python 2. Should avoid a copy on Python 3.xref: #372
TODO:
tox -e docs)