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

Adds Reponse.has_body. #259

Merged
merged 1 commit into from Jul 8, 2016
Merged

Adds Reponse.has_body. #259

merged 1 commit into from Jul 8, 2016

Conversation

martinth
Copy link

This is a result of the discussion in [https://github.com/Pylons/pyramid/issues/2625](Pyramid issue 2625).

Purpose of this property is to allow users to check if the Response has a body without forcing an evaulation of the underlying app_iter. This is handy if you wan't to create a streaming response where the app_iter itself is lazy (i.e. streams data over a socket) or the app_iter can only read once.

It's pretty much the same code as @bertjwregeer suggested but I added it as property rather than es method because I figured that it's fits better in the overall class design this way. The relevant changes for Pyramid will be done later.

Purpose of this property is to allow users to check if the Response has
a body without forcing an evaulation of the underlying app_iter.
This is handy if you wan't to create a streaming response where the
app_iter itself is lazy (i.e. streams data over a socket) or the app_iter
can only read once.
@digitalresistor
Copy link
Member

👍

@digitalresistor digitalresistor added this to the 1.7.0 milestone Jun 29, 2016
@mmerickel
Copy link
Member

This looks great. Just to bikeshed for a quick second... response.has_body could be construed to mean something like whether the status code says it should have a body or not. So I'm just curious if there's a more accurate name for this property that we could come up with. So far all I've got is... is_body_empty?

@martinth
Copy link
Author

You have a point. It could indeed cause confusion if you only look at the property name (which is something I totally have done in the past). is_body_empty is probably the most obvious choice for an alternative but I don't think it's a bad one.

@digitalresistor
Copy link
Member

I'd be okay with the rename. I could see the confusion happening...

@mmerickel
Copy link
Member

is_body_set ?

@mmerickel
Copy link
Member

I think I'm also fine with has_body. I think it's pretty easy to explain its behavior if there is any confusion.

@digitalresistor
Copy link
Member

There is currently no checking if the user attempts to send a Response with an HTTP code that does not allow for a body. This is something that I may want to change in the future...

I am okay with either one. is_body_set or has_body both work for me. I'll let it simmer for a little bit longer and then go ahead and merge it.

@digitalresistor digitalresistor merged commit d7ab300 into Pylons:master Jul 8, 2016
digitalresistor added a commit that referenced this pull request Jul 8, 2016
digitalresistor added a commit that referenced this pull request Jul 8, 2016
This adds a change to stop WebOb from reading the entire `app_iter` when
using the WebOb HTTP Exceptions.

Closes #259
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.

None yet

3 participants