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

[HttpKernel] Revert #45476 to fix missingRequest inRequestStack forStreamedResponse#51391

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

Conversation

@DaDeather
Copy link
Contributor

QA
Branch6.3
Bug fixyes
New featureno
Deprecationsno
TicketsFix#46743
LicenseMIT

Revert#45476 and add test to ensureRequest availability while usingStreamedResponse callback.

@alexander-schranz
Copy link
Contributor

alexander-schranz commentedAug 16, 2023
edited
Loading

Reverting#45476 will break streamedresponses for other Runtimes like Roadrunner, Swoole (php-runtime/runtime#115), is there any possibility todo this only when specificRuntime is used? /cc@nicolas-grekas.

Addition: The problem also is that it would behave differently between Roadrunner, Swoole, now as they could then still run into#46743 where this listener is required to be disabled. I understand@derrabus concern about#51139 but to have full support for such runtimes I don't see another way then make HttpKernel aware of a Response class specially it already has dependency to HttpFoundation.

andrew-demb reacted with heart emoji

@nicolas-grekas
Copy link
Member

Thanks for the reminder@alexander-schranz

@DaDeatherDaDeather deleted the revert-streamedresponselistener-deprecation branchAugust 16, 2023 08:36
@derrabus
Copy link
Member

derrabus commentedAug 16, 2023
edited
Loading

I understand@derrabus concern about#51139 but to have full support for such runtimes I don't see another way then make HttpKernel aware of a Response class specially it already has dependency to HttpFoundation.

The other way would be to document that the request stack is empty whenStreamedResponse's callback is executed. This is fine imho.

Note that the breaking change was to un-wire the deprecated listener which has happened without prior deprecation in a minor release. We could wire it again, but make it opt-out by introducing a config flag into FrameworkBundle. Yeah, I know how we don't like those.

@nicolas-grekas
Copy link
Member

document that the request stack is empty when StreamedResponse's callback is executed

This ship has sailed IMHO. Also it's very hard to deal with, because any services can be used in that callback. I think#51396 is pragmatic and makes sense also.

derrabus reacted with thumbs up emoji

@derrabus
Copy link
Member

I think#51396 is pragmatic and makes sense also.

LGTM.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

[HttpKernel] "Deprecation" of StreamedResponseListener causes RequestStack to be empty within StreamedResponse callback

5 participants

@DaDeather@alexander-schranz@nicolas-grekas@derrabus@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp