Fake streamed file response #60
Conversation
There was a problem hiding this comment.
We should test these new features by adding some controllers returning expected values and checking that the result is the expected one.
We should merge both commits in one as well.
Edit
What's the difference between this PR and #59?
src/RequestHandler.php
Outdated
| $response->sendContent(); | ||
| ob_end_flush(); | ||
| }); | ||
| } elseif($response instanceof StreamedResponse) { |
|
@Deltachaos Thanks for that PR. I've added some comments before merging. |
|
have updated the pr |
|
@Deltachaos we already added support for streams in drift. TBH, doesn't make sense to support any other stream implementation than the ReactPHP one, and as long as we can return a React HTTP Response object with a stream, I think that we are covered. Thanks for taking care of this. |
|
@mmoreram in general I would agree. On the other hand i have created #91 because what i think Drift should try to support is a mode where you can run an application using Drift in async mode, but also in a synchronous mode. This would make the adoption of Drift in existing projects more easy. For this we would require that streamed response implementations are compatible with classic PHP-FPM as well. |
2 participants
Depends on #59
There are two problems with symfony's
StreamedResponse:Symfony\Component\HttpKernel\EventListener\StreamedResponseListeneris unwrapping the response before it arrives in the kernel. By this, drift is not able to handle the response and output is not been send to the client.This PR handles the response by passing output to a BufferStream. From what i have seen, this is not really streaming the response anymore, and it is still not removing the blocking IO from symfony's StreamedResponse approach.
Problems that need to be solved are removing the
StreamedResponseListeneras it sends the contents before drift can send it. Any better implementation would require changes to the symfonyStreamedResponseclass.