-
Notifications
You must be signed in to change notification settings - Fork 55
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
Added ability to set a custom routing key per message #77
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@creyke - Thanks for the PR! Current design adds support to bind to byte[], which makes it easier for existing users to adopt functions. Tagged @jeffhollan and @fabiocav for further discussion on introducing a wrapper type
{ | ||
public class RabbitMQMessage | ||
{ | ||
public RabbitMQMessage(byte[] body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeffhollan / @fabiocav - Do you recommend introducing a wrapper type 'RabbitMQMessage'
Sorry to chase on this, is it possible to make this adjustment? While I understand this is a API change, I can't imagine many developers could use this library without a meta object, or a way to configure additional details pertaining to and around the message? |
@creyke apologies for the delayed response. We're reviewing the open PRs that have gone stale (not your fault) in the repo. Is this still relevant? Do you know if there's a specific reason why the SDK doesn't expose that? (sorry, haven't looked at the details, just assuming it would be needed there as well, unless it's provided as part of a method call). |
Hi @fabiocav, thanks for picking this up. I believe this has not been exposed yet due to the most basic of use cases not requiring it. Anyone using topic exchanges will require control over the routing key. If this can come in I'm confident many more customers can move over to this library, and it will not overly complicate the use cases for users which do not need to define it. |
As discussed in #51, it is an extremely common requirement to set a routing key of a message based on, for example it's content. The current implementation sets the routing key to the
QueueName
.This PR proposes an abstraction in the generic of the
IAsyncCollector
implementation which enables common meta data related to the message body to be expressed, in this case theRoutingKey
.This is an optional property, and is only used if it is explicitly defined, else normal operation resumes.