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

views: fix calls to make_response (API for REST views) #44

Open
lnielsen opened this issue Jun 24, 2016 · 3 comments
Open

views: fix calls to make_response (API for REST views) #44

lnielsen opened this issue Jun 24, 2016 · 3 comments

Comments

@lnielsen
Copy link
Member

lnielsen commented Jun 24, 2016

Problem:

When you define a view method in a subclass of ContentNegotiatedMethodView you can currently return (retval):

  • a response (no call to make_response())
  • a list/tuple which will call make_response(*retval)
  • anything else which will call make_response(retval)

See here

This causes confusion with Flask's make_response which behaves differently, and it also makes it hard to 1) get a consistent API for serialisers over many modules 2) change simple things like e.g. response code while keeping the serialiser the same.

Example:
GET/POST serialisation of an object is likely the same, but often response code will be 200 vs 201/202.

Proposals

  • A) Always just call make_response() explicitly from views
  • B) Align with Flask's make_response() and allow either 1) a response (already possible today) or 2) a 2/3-tuple of (response, status, headers) or (response, headers)
@lnielsen lnielsen added this to the v1.0.0 milestone Jun 24, 2016
@jirikuncar
Copy link
Member

@lnielsen can you provide an example how do you want to change the serializer API?

@hachreak
Copy link
Member

there are any news about this proposal?

@lnielsen
Copy link
Member Author

Proposal is still valid and I think should be fixed prior to final release. For both proposal A and B, we need to check all uses of ContentNegotiatedMethodView in other modules.

I would probably go with proposal A which involves:

  • Remove dispatch_request.
  • Update uses of ContentNegotiatedMethodView to always call self.make_response (this is usually what happens - see e.g. Records-REST and Files-REST).

@lnielsen lnielsen modified the milestones: v1.0.0, someday Feb 6, 2017
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

3 participants