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

[Messenger] use events consistently in worker#34217

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:4.4fromTobion:messenger-events
Nov 5, 2019

Conversation

Tobion
Copy link
Contributor

@TobionTobion commentedNov 1, 2019•
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#32560,#32614,#33843
LicenseMIT
Doc PR

The worker had the three ways to handle events

  1. $onHandledCallback inrun(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 aWorkerInterface 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 existingWorkerStoppedEvent it's very symmetrical and solves the referenced issues.

chalasr, onEXHovia, and Koc reacted with thumbs up emoji

/**
* @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 = [])
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

EventDispatcher is required to make the limit options reliable.

@TobionTobion added this to the4.4 milestoneNov 1, 2019
@TobionTobionforce-pushed themessenger-events branch 2 times, most recently from87db6f8 toffea7d3CompareNovember 1, 2019 22:28
@TobionTobion requested review fromweaverryan and removed request forsrozeNovember 1, 2019 22:28
private $logger;
private $memoryResolver;

public function __construct(int $memoryLimit, LoggerInterface $logger = null, callable $memoryResolver = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't it make sense to make the logger the third argument?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

The memory resolver is mainly for testing and it has been like this before.

OskarStark reacted with thumbs up emoji
Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Great work 👏

Copy link
Member

@weaverryanweaverryan left a comment

Choose a reason for hiding this comment

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

This is incredible. It's another very nice cleanup with no downside. Big +1 from me after the minor notes.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with minor comments)

@nicolas-grekas
Copy link
Member

Thank you@Tobion.

nicolas-grekas added a commit that referenced this pull requestNov 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 events1. $onHandledCallback in `run(array $options = [], callable $onHandledCallback = null)`2. events dispatched using the event dispatcher3. hardcoded things inside the workerThis 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
@nicolas-grekasnicolas-grekas merged commit201f159 intosymfony:4.4Nov 5, 2019
@TobionTobion deleted the messenger-events branchNovember 5, 2019 21:57
wouterj added a commit to symfony/symfony-docs that referenced this pull requestNov 9, 2019
This PR was merged into the 4.4 branch.Discussion----------[Messenger] update events for 4.4Documentation forsymfony/symfony#34217Commits-------3374261 [Messenger] update events for 4.4
This was referencedNov 12, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@pkruithofpkruithofpkruithof left review comments

@OskarStarkOskarStarkOskarStark left review comments

@chalasrchalasrchalasr left review comments

@fabpotfabpotfabpot approved these changes

@weaverryanweaverryanweaverryan approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.

[Messenger] Custom method argument $onHandledCallback for worker with command messenger:consume
8 participants
@Tobion@nicolas-grekas@fabpot@weaverryan@pkruithof@OskarStark@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp