[Messenger] use events consistently in worker#34217
Merged
nicolas-grekas merged 1 commit intosymfony:4.4from Nov 5, 2019
Merged
[Messenger] use events consistently in worker#34217nicolas-grekas merged 1 commit intosymfony:4.4from
nicolas-grekas merged 1 commit intosymfony:4.4from
Conversation
Tobion
commented
Nov 1, 2019
| * @param RoutableMessageBus $routableBus | ||
| */ | ||
| public function __construct($routableBus, ContainerInterface $receiverLocator, LoggerInterface $logger = null, array $receiverNames = [], /* EventDispatcherInterface */ $eventDispatcher = null) | ||
| public function __construct($routableBus, ContainerInterface $receiverLocator, EventDispatcherInterface $eventDispatcher, LoggerInterface $logger = null, array $receiverNames = []) |
Contributor
Author
There was a problem hiding this comment.
EventDispatcher is required to make the limit options reliable.
a25dffc to
fe147b6
Compare
87db6f8 to
ffea7d3
Compare
ffea7d3 to
ffbc91e
Compare
OskarStark
reviewed
Nov 2, 2019
src/Symfony/Component/Messenger/Command/FailedMessagesRetryCommand.php
Outdated
Show resolved
Hide resolved
OskarStark
reviewed
Nov 2, 2019
| private $logger; | ||
| private $memoryResolver; | ||
|
|
||
| public function __construct(int $memoryLimit, LoggerInterface $logger = null, callable $memoryResolver = null) |
Contributor
There was a problem hiding this comment.
wouldn't it make sense to make the logger the third argument?
Contributor
Author
There was a problem hiding this comment.
The memory resolver is mainly for testing and it has been like this before.
ffbc91e to
8b767a9
Compare
weaverryan
reviewed
Nov 3, 2019
Contributor
weaverryan
left a comment
There was a problem hiding this comment.
This is incredible. It's another very nice cleanup with no downside. Big +1 from me after the minor notes.
This was referenced Nov 3, 2019
fabpot
approved these changes
Nov 4, 2019
pkruithof
reviewed
Nov 5, 2019
src/Symfony/Component/Messenger/EventListener/StopWorkerOnSigtermSignalListener.php
Show resolved
Hide resolved
src/Symfony/Component/Messenger/EventListener/StopWorkerOnSigtermSignalListener.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/EventListener/StopWorkerOnMemoryLimitListener.php
Outdated
Show resolved
Hide resolved
8b767a9 to
9911a6d
Compare
weaverryan
approved these changes
Nov 5, 2019
9911a6d to
201f159
Compare
Member
|
Thank you @Tobion. |
nicolas-grekas
added a commit
that referenced
this pull request
Nov 5, 2019
This PR was merged into the 4.4 branch. Discussion ---------- [Messenger] use events consistently in worker | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #32560, #32614, #33843 | License | MIT | Doc PR | The worker had the three ways to handle events 1. $onHandledCallback in `run(array $options = [], callable $onHandledCallback = null)` 2. events dispatched using the event dispatcher 3. hardcoded things inside the worker This PR refactores the messenger worker to only use event dispatching. So instead of a hardcoded `$onHandledCallback` and worker decorators, we use event listeners and we don't need a `WorkerInterface` at all. The behavior of all the options like `--memory-limit` etc remains the same. I introduced two new events - `WorkerStartedEvent` - `WorkerRunningEvent` Together with the existing `WorkerStoppedEvent` it's very symmetrical and solves the referenced issues. Commits ------- 201f159 [Messenger] use events consistently in worker
wouterj
added a commit
to symfony/symfony-docs
that referenced
this pull request
Nov 9, 2019
This PR was merged into the 4.4 branch. Discussion ---------- [Messenger] update events for 4.4 Documentation for symfony/symfony#34217 Commits ------- 3374261 [Messenger] update events for 4.4
This was referenced Nov 12, 2019
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The worker had the three ways to handle events
run(array $options = [], callable $onHandledCallback = null)This PR refactores the messenger worker to only use event dispatching. So instead of a hardcoded
$onHandledCallbackand worker decorators, we use event listeners and we don't need aWorkerInterfaceat all. The behavior of all the options like--memory-limitetc remains the same.I introduced two new events
WorkerStartedEventWorkerRunningEventTogether with the existing
WorkerStoppedEventit's very symmetrical and solves the referenced issues.