Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't cache a ReturnDict. #2360

Closed
StevenVeshkini opened this issue Dec 27, 2014 · 14 comments
Closed

Can't cache a ReturnDict. #2360

StevenVeshkini opened this issue Dec 27, 2014 · 14 comments

Comments

@StevenVeshkini
Copy link

StevenVeshkini commented Dec 27, 2014

Has anyone been able to cache serializer.data from a custom Serializer on one object? Seems like there's no way to store the ReturnDict that results from serializer.data.

@tomchristie
Copy link
Member

tomchristie commented Dec 28, 2014

Could you also include a concrete use case to work against. In get the question generally, but it'd be helpful to see the specific case you're trying to achieve.

@tomchristie
Copy link
Member

tomchristie commented Dec 28, 2014

Also what error is raised?

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 30, 2014

It seems that the ReturnDict having a link to the serializer breaks the pickling:

>>> from rest_framework import serializers
>>> class NestedSerializer(serializers.Serializer):
...             one = serializers.IntegerField(max_value=10)
...             two = serializers.IntegerField(max_value=10)
... 
>>> class TestSerializer(serializers.Serializer):
...             nested = NestedSerializer()
... 
>>> input_data = {
...             'nested': {
...                 'one': 1,
...                 'two': 2,
...             }
...         }
>>> s = TestSerializer(input_data)
>>> s.data
ReturnDict([('nested', OrderedDict([('one', 1), ('two', 2)]))])
>>> d = s.data
>>> import pickle
>>> pickle.dumps(d)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
_pickle.PicklingError: Can't pickle <class 'TestSerializer'>: attribute lookup TestSerializer on builtins failed
>>> pickle.dumps({'nested': [{'one': 1, 'two': 2}]})
b'\x80\x03}q\x00X\x06\x00\x00\x00nestedq\x01]q\x02}q\x03(X\x03\x00\x00\x00twoq\x04K\x02X\x03\x00\x00\x00oneq\x05K\x01uas.'

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 30, 2014

Note that Django uses pickle to cache things.

@StevenVeshkini
Copy link
Author

StevenVeshkini commented Dec 30, 2014

@xordoquy Yes, that is the exact problem I am having (nested serializers).

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 30, 2014

It was a bit late yesterday. I'll add a proper test case this evening.

@StevenVeshkini
Copy link
Author

StevenVeshkini commented Dec 31, 2014

@xordoquy Is this a bug or was this the way DRF was designed?

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 31, 2014

It's both ;)

@StevenVeshkini
Copy link
Author

StevenVeshkini commented Dec 31, 2014

hahahaha nice :octocat: :P

@StevenVeshkini
Copy link
Author

StevenVeshkini commented Jan 3, 2015

Any luck?

@tomchristie
Copy link
Member

tomchristie commented Jan 5, 2015

So the right way to resolve this would be either...

  • Make serializer instances picklable, which oughta resolve this.

Or...

  • Make ReturnDict's pickle representation not include the serializer backlink. Given that we render the .content that oughta be okay in most cases anyways, not clear if accessing response.data from cached responses is something that should happen or not.

My personal take is that using pickle to deal with HTTP caching is a wonky approach. Rather store the raw headers and content, and return a newly created response instance when returning cached content. (Then you don't bump into all this horrible sorts of cases.)

@picturedots
Copy link

picturedots commented Jan 5, 2015

I encountered this problem (I think) when I was trying to set up caching
and gave up on this approach. I ended up caching the entier rendered
response instead, using the URL as the key, but only if the accepted
renderer wasn't api, i.e.
should_cache = self.request.accepted_renderer.format != 'api'

On Mon, Jan 5, 2015 at 12:31 PM, Tom Christie notifications@github.com
wrote:

So the right way to resolve this would be either...

  • Make serializer instances picklable, which oughta resolve this.

Or...

  • Make ReturnDict's pickle representation not include the serializer
    backlink. Given that we render the .content that oughta be okay in
    most cases anyways, not clear if accessing response.data from cached
    responses is something that should happen or not.

My personal take is that using pickle to deal with HTTP caching is a wonky
approach. Rather store the raw headers and content, and return a newly
created response instance when returning cached content. (Then you don't
bump into all this horrible sorts of cases.)


Reply to this email directly or view it on GitHub
#2360 (comment)
.

Mark Chackerian

VP Engineering, Architizer http://www.architizer.com/
646.812.0994

@thedrow
Copy link
Contributor

thedrow commented Jan 6, 2015

@tomchristie Sometimes you want different levels of caching. Especially in cases where your API has many reads.
First level is always the complete response. If it's not cached or invalidated then the serializer's output should be used.
I agree that in this specific case it's a better idea to save the final serialization result (usually JSON) but in most cases when you cache python objects it's better to use pickle in order to store the value in the cache.

@gaffney
Copy link
Contributor

gaffney commented Jan 7, 2015

I just encountered this issue as well while working on the upgrade. It also happens with a ReturnList:

(Error lines: cPickle.c #L2100-2106)

  subl /Users/_/.virtualenvs/_/lib/python2.7/site-packages/django/core/cache/backends/locmem.py:67
    pickled = pickle.dumps(value, pickle.HIGHEST_PROTOCOL)
PicklingError: Can't pickle <type 'generator'>: attribute lookup __builtin__.generator failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants