Skip to content
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

Feature Request: Multi exchange and multi queue binding #44

Open
falahati opened this issue Oct 2, 2021 · 2 comments
Open

Feature Request: Multi exchange and multi queue binding #44

falahati opened this issue Oct 2, 2021 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@falahati
Copy link
Contributor

falahati commented Oct 2, 2021

This feature request asks for the possibility of defining more than one exchange or queue and to allow selecting the right queue or exchange using the decorator. This allows for wider advanced usages like for example when a queue is used exclusively by an instance of the service for getting notifications while another is used for load balancing messages or when two exchanges has to be used for a single topic/message.
Implementing this feature also greatly decreases the need for non-global feature specific support for the library.

To achieve this and still be compatible with the older version I suggest adding a new queues and exchanges configurable properties with information about one or more exchanges or queues.

A bindingOption optional argument is then can be added to the decorator allowing it to gets binded to a all or one or more queues or exchanges.

channel.consume obviously has to change to account for this change but it is the only heavy part of this feature.

Are you ok with me creating a PR for this?

P.S. Been using this library in production with a dozen microservices happily for the last year, good job, and thank you.

@AlariCode
Copy link
Owner

AlariCode commented Oct 2, 2021

Thanks for support!
First we need to determine approach to this feature.

  1. The one that you described. Pros: backward compatibility, easy to code. Cons: NestJS module philosophy is not supported.
  2. We create forFeature() method, that declares name, queue, exchange and other parameters. And forRoot() only declare service names, logger and other common features. @RMQRoute gets additional parameter of module name, to bind correctly to the selected module. Pros: logic separation with modules, you can use different connections with different modules. Cons: no backward compatibility (I assume), more hard to implement.
    Second approach related to Non global module support #20

What do you think, would be better and why?

@AlariCode AlariCode added the enhancement New feature or request label Oct 2, 2021
@falahati
Copy link
Contributor Author

falahati commented Oct 2, 2021

I think adding the forFeature() is a must with or without this change. forFeature() allows dependency separation which might be quite useful in a modular sort of way, and as you said it, more in line with what NestJS philosophy dictates or is the norm.

However, this FR is not actually a replacement for the nonglobal modules, but rather helps decrease such needs indirectly by adding a new feature that could in the majority of cases used as a workaround. So from the modular point of view, no this feature is not ideal. However, the added functionalities are not going to be provided with the addition of forFeature anyway, so one can argue that this feature is completely separate.

But yeah, I suppose forFeature is more preferable. If we write it the way TypeORM does it we might be able to allow backward compatibility. But I have to double-check. I will take a look into this and keep you in the loop for any finding; might even submit a PR for forFeature. But in any case, if backward compatibility is broken, you can release a new major version, the migration for users would be easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants