-
Notifications
You must be signed in to change notification settings - Fork 54
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an AsyncSwitchMessageRouter #128
Comments
@codeliner I have written a router decorator that routes to an async provider or if its already been routed drops through to the decorated router to be handled. Before I finish the tests, please can you take a look as see if this is what you had in mind. also any suggestions gratefully relieved. I have used this in my project and seems to work as expected. interface...wonders if they should be in the Async folder? but also its part of the router. The switch router decorator Thank you |
@guyradford great work 👍 , exactly what I had in mind please, submit a PR as soon as your tests are ready. @prolic can take a second look then Can we remove the if statement here?: https://github.com/guyradford/service-bus/blob/master/src/Plugin/Router/AsyncSwitchMessageRouter.php#L79 and just do: if ($message instanceof AsyncMessage && !($message->metadata()['handled-by-async-queue'] ?? false))
Yes, you need to add the (new) message, because the message is immutable and And I'd love to see the Last but not least: a short description of the router would be nice here: https://github.com/guyradford/service-bus/blob/master/docs/plugins.md#routers Again, many many thanks for the awesome work! |
Great stuff. Only one thing besides those comments by @codeliner: |
One thing I don't really like is that I need all messages to be handled by the same producer with this router. As soon as I need different async message producers on the same bus it all makes no sense any more, because then I can route those messages directly to their producer in the bus config. |
good point @prolic
definitely a limitation, we should add this as a note in the docs. Using the router is optional so users can decide what they prefer. |
and maybe just |
@prolic If I understand correctly... because we only have one marker interface and that is hard coded in the router, then we are limited? We could pass in the interface to allow people to chose which interface trigger this? |
@guyradford the router knows only one message producer. that is the limitation. |
@codeliner exactly. Second: I like handled-async |
@codeliner @prolic Another thought...what happens when I add more then one router to the commandBus pluggins? Does it keep all? If so can I guarantee the order? Should this then be a Router, but then I guess it will also be handled by the other routers? |
then the Async Routers decorators could be stacked each one had its own marker and producer? |
@guyradford in theory you could work with priorites the ActionEventDispatcher is aware of such a concept but priorites add too much complexity. I'd say we keep this AsyncSwitchMessageRouter as simple as it is. I'm fine with the limitiation because normally you'll have one messaging system in place. If not, you should be very explicit with your message routing anyway and therefor use dedicated message -> message producer routes then. |
Agree with @codeliner |
Hi @codeliner & @prolic https://github.com/guyradford/service-bus/tree/async-router Tests done, please can you check before I do a PR Thank you |
Just submit the PR. It is easier to review then. If you need to change something you can simply commit and push to your branch again. The PR gets updated automatically. gesendet von meinem Nokia 3310 -------- Original message -------- Hi @codeliner & @prolic https://github.com/guyradford/service-bus/tree/async-router Tests done, please can you check before I do a PR Thank you — |
ok ta :) |
Added with #132 |
Use case taken from the chat:
Idea:
the
AsyncSwitchMessageRouter
can decorate a "standard router plugin" configured with the in-process message routes. The router should check either a specific message metadata key or if message implements anAsyncMessage
marker interface (tbd) and route the message to a configured message producer.In the draft from the chat the consumer adds a metadata key to tell the router that the message was consumed from a queue. But the router should add such a metada key BEFORE it routes the message to a producer. So the router can check its own metadata key when it receives the message a second time. If the metadata key is present the
AsyncSwitchMessageRouter
delegates routing to its inner (decorated) router.The text was updated successfully, but these errors were encountered: