betamaxpy / betamax

A VCR imitation designed only for python-requests.
https://betamax.readthedocs.io/en/latest/
Other
565 stars 62 forks source link

Add support for binary serializer storage #152

Closed sigmavirus24 closed 6 years ago

sigmavirus24 commented 6 years ago

Some serializers return bytes instead of text and we need a way to indicate that when opening the cassette file.

Related-to gh-123


cc @wimglenn for review

hroncok commented 6 years ago

The code looks reasonable. I would appreciate some kind of test tough.

sigmavirus24 commented 6 years ago

Thanks for the prompt @hroncok. I added them in the latest commit.

wimglenn commented 6 years ago

This "corrected file mode" method seems to be a needless complexity, I prefer my implementation where it's just using plain old class attributes on the serializer. However, it's the same basic functionality provided, so LGTM

sigmavirus24 commented 6 years ago

I prefer my implementation where it's just using plain old class attributes on the serializer.

There's something to be said for keeping things simpler for everyone. Your implementation is ideal if there's ever a case where a serializer wants to read text and write bytes or vice-versa, but that seems to be entirely unlikely. Simplifying what a serializer author needs to care about makes things harder to do wrong.