Skip to content

[Messenger] Add messenger configuration to make a message bus not get all handlers by default #42693

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

Open
wants to merge 1 commit into
base: 7.4
Choose a base branch
from

Conversation

Jeroeny
Copy link
Contributor

@Jeroeny Jeroeny commented Aug 23, 2021

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
License MIT
Doc PR not yet

When working with multiple Messenger buses, it's common to restrict a handler to certain bus(es) using the tag properties. This is useful if you have handlers that all belong to specific buses, but becomes more complicated when there are more combinations of (default and non-default) buses and handlers.

For example if you have a bus to handle new messages and want to introduce a bus that only handles a migrations process' messages in the background. If the handlers for "new messages" should keep handling only those, then you'd suddenly need to make that explicit for all those handlers, because otherwise they'd be added to the "migration process" bus too. And if a third party bundle also registers message handlers, you'd need to restrict those to the specific bus(es) too, if even possible.

This can be improved (IMHO) with a simple addition, by adding a configuration value for the messenger configuration (framework.messenger.buses[].default_handlers) that determines if the bus is added by default to handlers. This then stores this config in a tag property, which can be read in the MessengerPass. The change would be fully backwards compatible. I'd first like to know if this is in line with the Messenger component's way of working and something that could be added. I can add/fix tests, codestyle etc after discussion.

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@Jeroeny Jeroeny marked this pull request as ready for review August 23, 2021 15:00
@Jeroeny Jeroeny requested a review from sroze as a code owner August 23, 2021 15:00
@Jeroeny Jeroeny changed the title [Messenger ] Add messenger configuration to make a message bus not get all handlers by default [Messenger] Add messenger configuration to make a message bus not get all handlers by default Aug 23, 2021
@carsonbot
Copy link

Hey!

I think @ruudk has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@chalasr chalasr added this to the 5.4 milestone Sep 1, 2021
@fabpot fabpot modified the milestones: 5.4, 6.1 Nov 16, 2021
@fabpot fabpot modified the milestones: 6.1, 6.2 May 20, 2022
@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants