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

Fix JSONP issues #212

Closed
wants to merge 2 commits into from
Closed

Conversation

dracos
Copy link
Member

@dracos dracos commented Nov 6, 2015

An alternative solution to 2a30c9e would be for non-200 responses, consuming the generator immediately, as it'll always be small. Not sure if that's preferable/clearer.

If a callback variable was supplied for a view that errors (e.g. no
polygons), then this led to a cacheable (200) response which contained a
generator function (iterencode) in its _closable_objects attribute. The
cache could not then pickle this attribute and so raised an exception.

This was okay in previous versions because of a side effect of the
smart_bytes workaround for Django bug #24240 that was in place; this
was removed when support for Django prior to 1.8 was dropped.

Fix the issue by using an identity map (as map has no close() method and
so is not added to _closable_objects).
@stevenday
Copy link
Contributor

👍 I think 2a30c9e is as good as the alternative you suggest, since it can be used in all cases (200 or not).

Don't add callback to non-JSON responses, or to responses that already
have a callback (presumably returned from a cache).
@dracos dracos force-pushed the mapit-generator-pickle-middleware-fix branch from decd580 to 42032d3 Compare November 10, 2015 12:37
@dracos
Copy link
Member Author

dracos commented Nov 10, 2015

Merged at 66e8f90 (fixed a comment typo).

@dracos dracos closed this Nov 10, 2015
@dracos dracos removed the Reviewing label Nov 10, 2015
@dracos dracos deleted the mapit-generator-pickle-middleware-fix branch December 22, 2015 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants