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] extract worker logic to listener and get rid of SendersLocatorInterface::getSenderByAlias#34185

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
Tobion merged 1 commit intosymfony:4.4fromTobion:messenger-refactoring
Nov 1, 2019

Conversation

@Tobion
Copy link
Contributor

@TobionTobion commentedOct 30, 2019•
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?no
Deprecations?no
TicketsFix#32077 and#31848
LicenseMIT
Doc PR

as discussed with@weaverryan sending messages for retry and failure directly to transport instead of redispatching on the bus makes things much cleaner

@TobionTobion requested a review fromsroze as acode ownerOctober 30, 2019 11:20
@TobionTobion added this to the4.4 milestoneOct 30, 2019
@TobionTobionforce-pushed themessenger-refactoring branch 5 times, most recently from72f2480 to3a2672fCompareOctober 31, 2019 12:51
@TobionTobion changed the title[Messenger] WIP: refactoring[Messenger] extract worker logic to listener and get rid of SendersLocatorInterface::getSenderByAliasOct 31, 2019
useSymfony\Component\Messenger\Transport\Sender\SendersLocator;
useSymfony\Component\Messenger\Worker;

class RetryIntegrationTestextends TestCase
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

fully covered by FailureIntegrationTest

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.

Absolutely wonderful.

This removes some "evil" (added by me 👿) without any downside that I can see. It's simply a cleaner implementation - moving several "special cases" like theRedeliveryStamp handling - out of core classes likeWorker andSendMessageMiddleware and into event listeners.

and failure directly to transport instead of redispatching on the bus
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.

+1

Test failures match failures on current 4.4 or things that should resolve after merge afaik (like FWBundle from this PR failing against symfony/messenger:dev-master due to a class added in this PR missing).

Tobion added a commit that referenced this pull requestNov 1, 2019
…id of SendersLocatorInterface::getSenderByAlias (Tobion)This PR was merged into the 4.4 branch.Discussion----------[Messenger]  extract worker logic to listener and get rid of SendersLocatorInterface::getSenderByAlias| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       |Fix#32077 and#31848| License       | MIT| Doc PR        |as discussed with@weaverryan sending messages for retry and failure directly to transport instead of redispatching on the bus makes things much cleanerCommits-------d7e0f98 [Messenger] extract worker logic to listener and sent messages for retry and failure directly to transport instead of redispatching on the bus
@TobionTobion merged commitd7e0f98 intosymfony:4.4Nov 1, 2019
@TobionTobion deleted the messenger-refactoring branchNovember 1, 2019 23:49
@nikic
Copy link
Contributor

The testSymfony\Component\Messenger\Tests\FailureIntegrationTest::testRequeMechanism hangs locally after this change.

@Tobion
Copy link
ContributorAuthor

It's getting fixed in#34217

@HypeMCHypeMC mentioned this pull requestOct 19, 2025
nicolas-grekas added a commit that referenced this pull requestOct 20, 2025
This PR was merged into the 7.4 branch.Discussion----------[Messenger] Simplify code| Q             | A| ------------- | ---| Branch?       | 7.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITNoticed while working on#61843.There's no reason for the event to be dispatched inside the loop anymore:1. The event was [originally dispatched outside the loop](#30650).2. It was later [moved inside the loop](#30676) because of the retry mechanism.3. [The retry mechanism was later extracted](#34185), so there's no longer a need for the event to be dispatched inside the loop.Commits-------beeb4b9 [Messenger] Simplify code
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@weaverryanweaverryanweaverryan approved these changes

@srozesrozeAwaiting requested review from sroze

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

[Messenger] refactoring to get rid of SendersLocatorInterface::getSenderByAlias

4 participants

@Tobion@nikic@weaverryan@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp