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

[Swoole] Send StreamedResponse #151

Closed
wants to merge 4 commits into from
Closed

Conversation

barryvdh
Copy link
Contributor

@barryvdh barryvdh commented Apr 9, 2021

Capture the output from a StreamdResponse. Fixes #109

Note that this doesn't actually stream the contents anymore, but it does at least render the output (so after the callback is finished).

@themsaid
Copy link
Member

themsaid commented Apr 9, 2021

@barryvdh what do you think of #153 ?

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 9, 2021

So capture all output? Technically I think that a proper response object is always the way to go, but I understand the appeal of dump(), and it's closer to 'normal'

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 9, 2021

So based on the comment, the previous if statement doesn't actually do anything ever. I couldn't really see where the output property was coming from, but as I gather from #109 (comment) that based on laravel-swoole, who do use that. I think it's safe to remove here?

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 9, 2021

(RoadRunner was already working, the PSR-7 factory does something similar, so just added a test)

@albertcht
Copy link
Contributor

So based on the comment, the previous if statement doesn't actually do anything ever. I couldn't really see where the output property was coming from, but as I gather from #109 (comment) that based on laravel-swoole, who do use that. I think it's safe to remove here?

I think removing the first statement for stream response is safe.

@taylorotwell
Copy link
Member

Will follow up with this on #153 ... thanks!

@Radiergummi
Copy link

Hey there @barryvdh ,

Note that this doesn't actually stream the contents anymore, but it does at least render the output (so after the callback is finished).

So, I tried to follow the trails through the issues but couldn't find directions or documentation on stream responses using Octane. Is this generally impossible? If so, that would make a great addition to the documentation, and it's a real headache for our application :( Is there any recommended strategy, any idea?
We have this data export endpoint that streams hundreds of megabytes of CSV reports...

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.

Response doesn't support stream-based output
5 participants