Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpClient] Remove final keyword onAsyncResponse#50577
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
stof commentedJun 6, 2023
Doesn't this risk breaking concurrency features of support for streaming chunks ? The override of headers is probably the only use case for which extending AsyncResponse is aworking solution. So if we don't make AsyncResponse final anymore, maybe a bunch of its methods should become final instance, to prevent overriding them by thinking that it will work while it would break half of the ways the client can be used. Any of the methods dealing with the content cannot be used to implement additional logic without breaking the streamed usage. |
nicolas-grekas commentedJun 6, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Let's make getContent |
lyrixx commentedJun 7, 2023
@nicolas-grekas done ✅ |
AsyncResponsesrc/Symfony/Component/HttpClient/Response/CommonResponseTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedJun 9, 2023
Thank you@lyrixx. |
In symfony < 6.3, it was possible to override HTTP response headers. See thisblog post for a use case
With Symfony 6.3, this is not possible anymore. I created a reproducer here:https://github.com/lyrixx/test/tree/http-cache-pp
And@nicolas-grekas came with another solution:lyrixx/test#11
So here we go!