Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
nicolas-grekas merged 1 commit intosymfony:6.3fromlyrixx:http-client-no-final
Jun 9, 2023

Conversation

@lyrixx
Copy link
Member

QA
Branch?6.3
Bug fix?kind of
New feature?no
Deprecations?no
Tickets
LicenseMIT
Doc PR

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!

@stof
Copy link
Member

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
Copy link
Member

nicolas-grekas commentedJun 6, 2023
edited
Loading

Let's make getContent, toArray and toStream final? The rest should be fine being extended.

stof reacted with thumbs up emoji

@lyrixx
Copy link
MemberAuthor

@nicolas-grekas done ✅

@lyrixxlyrixxforce-pushed thehttp-client-no-final branch fromcf01e19 todc727d3CompareJune 7, 2023 08:46
@OskarStarkOskarStark changed the title[HttpClient] Remove final keyword on AsyncResponse[HttpClient] Remove final keyword onAsyncResponseJun 7, 2023
@lyrixxlyrixxforce-pushed thehttp-client-no-final branch fromdc727d3 tofbd6d18CompareJune 9, 2023 09:04
@nicolas-grekas
Copy link
Member

Thank you@lyrixx.

@nicolas-grekasnicolas-grekas merged commitd9a8902 intosymfony:6.3Jun 9, 2023
@lyrixxlyrixx deleted the http-client-no-final branchJune 9, 2023 09:07
@fabpotfabpot mentioned this pull requestJun 26, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.3

Development

Successfully merging this pull request may close these issues.

4 participants

@lyrixx@stof@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp