Fix unpickling in Python 3 #353
Conversation
stripe/resource.py
Outdated
| def __setstate__(self, state): | ||
| self.update(state) | ||
|
|
||
| def __reduce__(self): |
There was a problem hiding this comment.
Can we do comments on both __setstate__ and __reduce__ that follow roughly exactly what you said in the PR description?
There was a problem hiding this comment.
(Someone reading this who didn't see the original PR might wonder what exactly is going on here.)
| obj.refresh_from( | ||
| { | ||
| 'fala': 'lalala', | ||
| 'emptystring': '', |
There was a problem hiding this comment.
Can we put a comment here explaining that emptystring is trying to uncover a specific Python 3 unpickling issue?
|
Thanks for tracking this down OB!
Yeah, the dict subclass thing is so brutal. We should definitely consider squashing that at some point. |
|
Added some comments as requested. |
|
Excellent. Thanks! |
r? @brandur-stripe
cc @stripe/api-libraries
Fixes #352.
This was annoying :/
Basically, the issue is that when unpickling
StripeObjects, Python 3 calls our custom__setitem__method which forbids empty strings.In order to directly reset the dict values, it is necessary to create a custom
__setstate__method that will useupdateinstead.However, because
StripeObjectis a subclass ofdict,__setstate__is not called automatically when unpickling the instance, and I had to define a__reduce__method to force the pickler to treat the instance as a custom class and not as a normal dict.I'm not super happy with this fix as it's relatively brittle (
__reduce__needs to be consistent with the class constructor) but it's an unfortunate consequence of havingStripeObjectdirectly subclassdict. I wish we had gone with composition over inheritance here, but that's another discussion...