piston-mini-client

Merge lp://staging/~elachuni/piston-mini-client/remove-base-serializer into lp://staging/piston-mini-client

  1. remove-base-serializer
  2. Merge into trunk
Proposed by Anthony Lenton
Status: Merged
Merged at revision: 27
Proposed branch: lp://staging/~elachuni/piston-mini-client/remove-base-serializer
Merge into: lp://staging/piston-mini-client
Diff against target: 171 lines (+40/-41)
4 files modified
piston_mini_client/__init__.py (+6/-3)
piston_mini_client/serializers.py (+17/-24)
piston_mini_client/test/test_resource.py (+4/-2)
piston_mini_client/test/test_serializers.py (+13/-12)
To merge this branch: bzr merge lp://staging/~elachuni/piston-mini-client/remove-base-serializer
Reviewer Review Type Date Requested Status
Michael Nelson Approve
Review via email: mp+48733@code.staging.launchpad.net

Description of the change

Overview
========
This branch provides two small improvements:
 - it removes the BaseSerializer class, and
 - it adds a "X-Requested-With: XMLHttpRequest" header to all POST and PUT requests.

Details
=======
The BaseSerializer class ultimately served no purpose. It stored the object that the serializer was initialized with, but this isn't really necessary as the serializers can be completely stateless and just receive an object as an argument to serialize()

The X-Requested-With header is the recommended way to avoid csrf on all api calls without needing to explicitly add csrf_exempt decorators to all your api resources.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

The change looks straight-forward except for one point, which you cleared up via irc

{{{
09:05 < noodles> achuni: is:
09:05 < noodles> serializer = get_serializer(content_type)
09:05 < noodles> returning a class, or an instance?
09:05 < noodles> The diff makes it look like it's a class.
09:06 < noodles> (ie. as if it should be get_serializer(content_type)() )
09:06 < achuni> it was a class
09:06 < achuni> now it should be an instance
09:07 < noodles> k... all good then, with that note :)
09:07 < achuni> (changed on lines 85..88 of the diff)
09:08 < achuni> as serializers are stateless now, there's no harm in keeping instances around
09:08 < noodles> ah right. That makes sense.
09:08 < achuni> (also get_serializer(foo)(data).serialize() was ugly :) )
09:08 < noodles> Yep
}}}

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches