NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Messenger] make StopWorkerOnSignalsListener listen by default on SIGTERM and SIGINT#49539

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

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:6.3fromlyrixx:messenge-signal

Mar 10, 2023

Merged

[Messenger] make StopWorkerOnSignalsListener listen by default on SIGTERM and SIGINT#49539

fabpot merged 1 commit intosymfony:6.3fromlyrixx:messenge-signal

Mar 10, 2023

Conversation

Copy link

Member

lyrixx commentedFeb 26, 2023•
edited
Loading

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	Fix#49156
License	MIT
Doc PR	no need I guess

lyrixx requested a review fromchalasr as acode owner

February 26, 2023 17:40

carsonbot added Status: Needs Review Deprecation Feature Messenger labels

Feb 26, 2023

carsonbot added this to the6.3 milestone

Feb 26, 2023

lyrixx force-pushed themessenge-signal branch 2 times, most recently from6430e04 to3759eacCompare

February 28, 2023 16:22

Copy link

MemberAuthor

lyrixx commentedMar 2, 2023

This PR is ready for review 👍🏼

Copy link

carsonbot commentedMar 2, 2023

Hey!

I think@ruudk has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

nicolas-grekas reviewed

Mar 9, 2023

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.php OutdatedShow resolvedHide resolved

lyrixx force-pushed themessenge-signal branch from3759eac toec409e6Compare

March 9, 2023 17:10

[Messenger] make StopWorkerOnSignalsListener listen by default on SIG…

9415b43

…TERM and SIGINT

lyrixx force-pushed themessenge-signal branch fromec409e6 to9415b43Compare

March 9, 2023 17:12

nicolas-grekas approved these changes

Mar 9, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Mar 9, 2023

nicolas-grekas added the Ready label

Mar 10, 2023

fabpot approved these changes

Mar 10, 2023

View reviewed changes

Copy link

Member

fabpot commentedMar 10, 2023

Thank you@lyrixx.

fabpot mentioned this pull request

Mar 10, 2023

[Messenger] make StopWorkerOnSignalsListener listen by default on SIGTE…symfony/symfony-docs#18040

Closed

fabpot merged commit550b92f intosymfony:6.3

Mar 10, 2023

lyrixx deleted the messenge-signal branch

March 10, 2023 13:45

fabpot mentioned this pull request

May 1, 2023

Release v6.3.0-BETA1#50199

Merged

HypeMC mentioned this pull request

Jun 27, 2023

[Messenger] Fix exitingmessenger:failed:retry command#50787

Merged

xabbuh mentioned this pull request

Jul 13, 2023

[Messenger] do not listen to signals if the pcntl extension is missing#50963

Merged

nicolas-grekas added a commit that referenced this pull request

Sep 29, 2023

bug#50787[Messenger] Fix exitingmessenger:failed:retrycommand (…

0539ca8

…HypeMC)This PR was merged into the 6.3 branch.Discussion----------[Messenger] Fix exiting `messenger:failed:retry` command| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -#49539 introduced a bug where it's impossible to exit the `messenger:failed:retry` command:![Screenshot](https://github.com/symfony/symfony/assets/2445045/6d6d271b-b5f6-4d2f-a150-847ead22083b)`Ctrl+C` doesn't work because the `StopWorkerOnSignalsListener` handles the signal but doesn't actually exit the command, so the only way to currently exit the command is to kill it by force.Commits-------cd6816b [Messenger] Fix exiting `FailedMessagesRetryCommand`