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

New test: setting response_body to a Proc should be supported. #446

Merged
merged 1 commit into from May 10, 2011
Merged

New test: setting response_body to a Proc should be supported. #446

merged 1 commit into from May 10, 2011

Conversation

danstutzman
Copy link

It looks like commit a66c917 "Do not inherit from Rack::Response, remove a shit-ton of unused code." removed the possibility for the user to set response_body to a Proc. Here is a test that demonstrates this behavior working in 3.0.7 and breaking in edge.
@ecoffey

@josevalim
Copy link
Contributor

Cool, thanks! There were no tests for it before, so I will bring the feature back as others seem to use it. However, I plan to deprecate it, can you please tell me what is your use case?

@danstutzman
Copy link
Author

I don't use this feature and don't mind if it's deprecated. Eoin and I were at a bugmash trying to fix https://rails.lighthouseapp.com/projects/8994/tickets/6026-patch-response_body-proc-is-called-twice .

@ecoffey
Copy link

ecoffey commented May 8, 2011

Our theory was that it was broken in favor of the new HTTP streaming features. Is that why it is also going to be deprecated?

@josevalim
Copy link
Contributor

@ecoffey it was removed because setting a proc to response_body never really worked. It stopped streaming since Rails 2.2 or 2.3 because of the Rack middleware stack.

@ghost ghost assigned josevalim May 8, 2011
@ecoffey
Copy link

ecoffey commented May 8, 2011

@josevalim gotcha. Well at least Daniel and I got a nice tour of the inner workings of actionpack dispatch :-)

josevalim added a commit that referenced this pull request May 10, 2011
New test: setting response_body to a Proc should be supported.
@josevalim josevalim merged commit 4d5ce47 into rails:master May 10, 2011
matthewd pushed a commit that referenced this pull request Apr 24, 2018
use #data_source_exists? if possible instead of deprecated #table_exi…
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