Skip to content

Allow rest_framework.request.Request instances #5724

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

sigmavirus24
Copy link
Contributor

Instead of flat-out erroring (which can cause unhandled exceptions)
let's perhaps be nicer to users who are considerately and carefully
reusing ViewSets in other ViewSets. This means accessing the private
attribute for them and then (and only then) raising an AssertionError if
it isn't a Django HttpResponse instance.

See also #5618

Instead of flat-out erroring (which can cause unhandled exceptions)
let's perhaps be nicer to users who are considerately and carefully
reusing ViewSets in other ViewSets. This means accessing the private
attribute for them and then (and only then) raising an AssertionError if
it isn't a Django HttpResponse instance.

See also encode#5618
@sigmavirus24
Copy link
Contributor Author

cc @carltongibson

@carltongibson
Copy link
Collaborator

OK, test suite is failing.

@rpkilby
Copy link
Member

rpkilby commented Jan 5, 2018

I'm definitely open to discussion, but my off the cuff reaction to this is -1.

In general, request processing is complicated. Depending on setup, there are cases where the underlying Django request parses the incoming stream, and other cases where the DRF request object parses the stream. Processing the stream multiple times seems like it would be both inefficient and prone to error.

As it currently stands, the assertion is a sanity check.

@carltongibson
Copy link
Collaborator

I'm going to say No to this as is.

I'm not 100% clear on the use-case. Something to do with re-use. I'm all for that but I suspect if you're bumping into errors here it's because you're not taking sufficient care over cloning the request before passing it off to another view.

(I'm a wider point, my first approach would be to extract the (business) logic from the view such that I didn't need to leverage the whole request/response machinery in order to re-use it.)

If there's a need to follow-up here I suggest a discussion on the mailing list to see if there's a work-around for a current use-case. If we can find a genuine limitation there then we can revisit as a new issue here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants