8000 [FrameworkBundle] Make StopWorkerOnSignalsListener configurable via messenger's config by rmikalkenas · Pull Request #49702 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] Make StopWorkerOnSignalsListener configurable via messenger's config #49702

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.

Sign up for GitHub < 8000 /a>

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 20, 2023
Merged

[FrameworkBundle] Make StopWorkerOnSignalsListener configurable via messenger's config #49702

merged 1 commit into from
Mar 20, 2023

Conversation

rmikalkenas
Copy link
Contributor
Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR Will prepare if PR gets merged

At the moment Symfony\Component\Messenger\EventListener\StopWorkerOnSignalsListener has hardcoded supported signals: SIGTERM, SIGINT. It's possible to pass custom ones, but it requires a compiler pass class which would override arguments. Adding a possibility to define it via messenger's config would make such configuration more easy.

cc @lyrixx

@carsonbot carsonbot added this to the 6.3 milestone Mar 16, 2023
@rmikalkenas rmikalkenas force-pushed the configurable-signals-listener branch from 624b734 to 3c29da5 Compare March 16, 2023 11:27
lyrixx
lyrixx reviewed Mar 16, 2023
View reviewed changes
Copy link
Member
@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

It makes sens to me👍🏼

@rmikalkenas rmikalkenas force-pushed the configurable-signals-listener branch from 3c29da5 to 1808920 Compare March 16, 2023 16:17
lyrixx
lyrixx approved these changes Mar 16, 2023
View reviewed changes
Copy link
Member
@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼 for the code. However, I'm not sure about the wording

nicolas-grekas
nicolas-grekas reviewed Mar 17, 2023
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Outdated
@@ -1526,6 +1526,11 @@ function ($a) {
->thenInvalid('The "framework.messenger.reset_on_message" configuration option can be set to "true" only. To prevent services resetting after each message you can set the "--no-reset" option in "messenger:consume" command.')
->end()
->end()
->arrayNode('stop_worker_on_signals')
->defaultValue([])
->info('A list of signals to stop worker. If not set - use defaults defined in listener.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be explicit here instead of referencing the listener

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas updated with clarification of default signals

@rmikalkenas rmikalkenas force-pushed the configurable-signals-listener branch from 1808920 to 2584c93 Compare March 20, 2023 13:06
@nicolas-grekas nicolas-grekas force-pushed the configurable-signals-listener branch from 2584c93 to 9ca9666 Compare March 20, 2023 13:14
@nicolas-grekas nicolas-grekas force-pushed the configurable-signals-listener branch from 9ca9666 to 245d75d Compare March 20, 2023 13:16
@nicolas-grekas
Copy link
Member

Thank you @rmikalkenas.

@nicolas-grekas nicolas-grekas mentioned this pull request Mar 20, 2023
Closed
@nicolas-grekas nicolas-grekas merged commit 1f43acb into symfony:6.3 Mar 20, 2023
@rmikalkenas rmikalkenas deleted the configurable-signals-listener branch March 20, 2023 18:40
@alexandre-daubois alexandre-daubois mentioned this pull request Mar 20, 2023
Merged
nicolas-grekas added a commit that referenced this pull request Feb 11, 2025
@nicolas-grekas
feature #49750 [FrameworkBundle] Allow to pass signals to `StopWorker…
4b6f05d 
…OnSignalsListener` in XML config and as plain strings (alexandre-daubois)

This PR was merged into the 7.3 branch.

Discussion
----------

[FrameworkBundle] Allow to pass signals to `StopWorkerOnSignalsListener` in XML config and as plain strings

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | _NA_
| License       | MIT
| Doc PR        | Todo

Follow-up of #49702 to improve DX. This PR allows to provide signals this way:

```yaml
framework:
    messenger:
        transports:
            async: '%env(MESSENGER_TRANSPORT_DSN)%'

        stop_worker_on_signals:
            - SIGINT
            - SIGUSR1
```

XML:

```xml
<?xml version="1.0" encoding="UTF-8" ?>
<container xmlns="http://symfony.com/schema/dic/services"
           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
           xmlns:framework="http://symfony.com/schema/dic/symfony"
           xsi:schemaLocation="http://symfony.com/schema/dic/services
        https://symfony.com/schema/dic/services/services-1.0.xsd
        http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

    <framework:config>
        <framework:messenger ...>
            <framework:stop-worker-on-signal>SIGINT</framework:stop-worker-on-signal>
            <framework:stop-worker-on-signal>SIGTERM</framework:stop-worker-on-signal>
            <framework:stop-worker-on-signal>SIGUSR1</framework:stop-worker-on-signal>
            <framework:stop-worker-on-signal>123</framework:stop-worker-on-signal>
        </framework:messenger>
    </framework:config>
</container>
```

Commits
-------

cd91c11 [FrameworkBundle] Add support for signal plain name in the `messenger.stop_worker_on_signals` configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@lyrixx lyrixx lyrixx approved these changes

Assignees
No one assigned
Labels
Feature FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
4 participants
@rmikalkenas @nicolas-grekas @lyrixx @carsonbot
0