[Messenger] Allows to register handlers on a specific transport #30958
Conversation
|
I see some drawbacks with current configuration:
Please consider move out queue name from dsn and extend routing configuration: framework:
messenger:
transports:
events: amqp://guest:guest@127.0.0.1/%2f/events
routing:
App\Message\RegisterBet:
- transport: events
queue: events_projection
handler: App\MessageHandler\RegisterBetHandler
- transport: events
queue: events_3rdparty
handler: App\MessageHandler\SomethingOnBetRegister
Handler can be omited for cases when we just pulishing messages and don't comsuming they through Messenger. This kind of config is super easy to understand. We have configuration of message-transport/queue-handler in single place. Of course |
106be1d to
1de1cb2
Compare
|
@Koc I don't see your point as being against this proposed implementation here. The main point is to create this In the example given, the handler indeed knows about the transport (it's already knowing about things like the Regarding the overhead in terms of configuring the exchange and the queue bindings, you could also use the |
1de1cb2 to
af34fbb
Compare
|
(requires CHANGELOG when a few 👍 will be around 😉 ) |
|
Open question: should we only have |
|
Makes sense to me - we do this with the bus name stamp and sender name for retry. |
af34fbb to
83758b6
Compare
|
@nicolas-grekas @weaverryan updated 👍 |
83758b6 to
cc6f13d
Compare
There was a problem hiding this comment.
This looks good to me - and I tested it locally.
There's just one piece that bothers me:
public static function getHandledMessages(): iterable
{
yield RegisterBet::class => [
'transport' => 'events_projection',
];
}The meaning of transport is this:
This handler will only be called when the message is received from the
events_projectiontransport
But it's just as reasonable to think it means this:
This handler will be sent to the
events_projectiontransport
But, the only possibly better key I can think of is: 'only_handle_from_transport' => 'events_projection',
src/Symfony/Component/Messenger/DependencyInjection/MessengerPass.php
Outdated
Show resolved
Hide resolved
Maybe |
|
Hmm, I do kinda like |
|
Happy with |
cc6f13d to
271c12b
Compare
|
This PR should be ready :) |
…his handler alias)
271c12b to
f0b2acd
Compare
…transport (sroze) This PR was merged into the 4.3-dev branch. Discussion ---------- [Messenger] Allows to register handlers on a specific transport | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30110 | License | MIT | Doc PR | symfony/symfony-docs#11236 With the #30008 pull-request in, we can now do the following: ```yaml framework: messenger: transports: events: dsn: amqp://guest:guest@127.0.0.1/%2f/events options: queues: 3rdparty: binding_keys: [ 3rdparty ] projection: binding_keys: [ projection ] events_3rdparty: amqp://guest:guest@127.0.0.1/%2f/events?queues[3rdparty] events_projection: amqp://guest:guest@127.0.0.1/%2f/events?queues[projection] routing: 'App\Message\RegisterBet': events ``` This will bind two queues to the `events` exchange, fantastic: <img width="325" alt="Screenshot 2019-04-07 at 10 26 27" src="https://user-images.githubusercontent.com/804625/55680861-af373580-591f-11e9-8f1e-2d3b6ddba2fd.png"> --- Now, in this setup, the message will be duplicated within the `3rdparty` & `projection` queues. If you just run the consumer for each transport, it will consume the message and call all the handlers. You can't do different things based on which queue you have consumed the message. **This pull-request adds the following feature:** ```php class RegisterBetHandler implements MessageSubscriberInterface { public function __invoke(RegisterBet $message) { // Do something only when the message comes from the `events_projection` transport... } /** * {@inheritdoc} */ public static function getHandledMessages(): iterable { yield RegisterBet::class => [ 'from_transport' => 'events_projection', ]; } } ``` --- In the debugger, it looks like this: <img width="649" alt="Screenshot 2019-04-07 at 10 29 55" src="https://user-images.githubusercontent.com/804625/55680892-1d7bf800-5920-11e9-80f7-853f0201c6d8.png"> Commits ------- f0b2acd Allows to register handlers on a specific transport (and get rid of this handler alias)
… from original sender (weaverryan) This PR was squashed before being merged into the 4.3 branch (closes #31425). Discussion ---------- [Messenger] On failure retry, make message appear received from original sender | Q | A | ------------- | --- | Branch? | master (4.3) | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #31413 | License | MIT | Doc PR | symfony/symfony-docs#11236 Fixes a bug when using the transport-based handler config from #30958. This also adds a pretty robust integration test that dispatches a complex message with transport-based handler config, failures, failure transport, etc - and verifies the correct behavior. Cheers! Commits ------- 80b5df2 [Messenger] On failure retry, make message appear received from original sender
|
The I still can't believe we use arbitrary array values in the suscriber instead of a value object that is self-documenting and can have types. There is even a |
…(sroze) This PR was merged into the 4.3 branch. Discussion ---------- [Messenger] Add `from_transport` in subscriber's phpdoc | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | ø | BC breaks? | ø | Deprecations? | ø | Tests pass? | yes | Fixed tickets | #30958 | License | MIT | Doc PR | ø Add the `from_transport` to the subscriber's phpdoc as it was missed from the original PR. Commits ------- 6a542cd Add transport in subscriber's phpdoc
This PR was merged into the 4.3 branch. Discussion ---------- Remove info about the HandlersLocator alias It seems that the option to provide an alias to the `HandlersLocator` was removed in symfony/symfony#30958 , but there are still some mentions of it in the documentation. Commits ------- fe8dc43 Remove info about the HandlersLocator alias
|
Please add the wonderful example from @sroze to the documentation on https://symfony.com/doc/current/messenger.html as a how to, took me some time to find this pull request. :) |
With the #30008 pull-request in, we can now do the following:
This will bind two queues to the
eventsexchange, fantastic:Now, in this setup, the message will be duplicated within the
3rdparty&projectionqueues. If you just run the consumer for each transport, it will consume the message and call all the handlers. You can't do different things based on which queue you have consumed the message. This pull-request adds the following feature:In the debugger, it looks like this:
