Add support for streamed response #50
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It seems like Travis is happy but fabbot.io failed with "Not Found" 🤔 |
dunglas
approved these changes
Nov 20, 2018
|
@dunglas done :) |
xthiago
approved these changes
Nov 22, 2018
jvasseur
reviewed
Nov 23, 2018
Factory/HttpFoundationFactory.php
Outdated
| @@ -140,11 +154,12 @@ protected function getTemporaryPath() | |||
| */ | |||
| public function createResponse(ResponseInterface $psrResponse) | |||
| { | |||
| $response = new Response( | |||
| $psrResponse->getBody()->__toString(), | |||
| $response = new StreamedResponse( | |||
There was a problem hiding this comment.
I don't think we should use stream responses in every possible cases because StreamedResponse have some downsides like preventing the debug toolbar from being injected.
Can this be made configurable ?
|
What about this PR ? Is it still relevant ? This is a nice feature to add in this component. |
|
Maybe by creating a another method ? Don't touch |
|
@danizord do you need help to finish this PR ? |
nicolas-grekas
force-pushed
the
feature/streamed-response
branch
from
0c60565 to
013db27
Compare
nicolas-grekas
force-pushed
the
feature/streamed-response
branch
from
013db27 to
7cc1605
Compare
|
I rebased the PR and made the behavior opt-in by adding a new |
nicolas-grekas
approved these changes
Mar 11, 2019
Nyholm
approved these changes
Mar 11, 2019
|
Thank you @danizord. |
nicolas-grekas
added a commit
that referenced
this pull request
Mar 11, 2019
This PR was merged into the 1.2-dev branch. Discussion ---------- Add support for streamed response Here's a simple implementation of response streaming heavily inspired by Diactoros' response emitter, using HttpFoundation's `StreamedResponse`. It lacks support for `Content-Range`, but I can provide that in a future PR. Closes https://github.com/symfony/psr-http-message-bridge/issues/3 Commits ------- 7cc1605 Add support for streamed response
nicolas-grekas
added a commit
that referenced
this pull request
Nov 25, 2019
This PR was squashed before being merged into the 1.2-dev branch. Discussion ---------- Add support for streamed Symfony request As a continuation of #3 and #50 . Add support for converting a `ServerRequestInterface` with a large body to a Symfony request. I'm not all too familiar with Symfony components, but I saw that a Symfony request supports `string|resource|null` as its body. Since calling `detach()` on a `StreamInterface` will make the stream unsuable I added a `$streamed` argument to the function in order to not make break any existing code, and to make it a conscious decision of the caller to stream the response. I'm happy to make any necessary changes if needed. Commits ------- df26630 Add support for streamed Symfony request
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a simple implementation of response streaming heavily inspired by Diactoros' response emitter, using HttpFoundation's
StreamedResponse. It lacks support forContent-Range, but I can provide that in a future PR.Closes https://github.com/symfony/psr-http-message-bridge/issues/3