krakend / krakend-amqp

AMQP compatible backend for the KrakenD framework
https://www.krakend.io
Apache License 2.0
10 stars 16 forks source link

Support static routing_key #22

Closed lennondtps closed 1 year ago

lennondtps commented 1 year ago

This Pull Request makes the routing keys work with static values, as mentioned in issue #21

How it works If the configuration of the routing key starts with an uppercase letter, it uses parameter keys from the endpoint URL Otherwise, it is used as a static value.

Routing key documentation: https://www.krakend.io/docs/backends/amqp-producer/#routing_key

lennondtps commented 1 year ago

Hello @alombarte @kpacha

Could you please review my PR if you have some time?

kpacha commented 1 year ago

ouch! I reviewed it 2 weeks ago and forgot to click the submit button...

I think it would be better to add a new configuration parameter instead of checking if the value starts with an uppercase, because you may want a static routing key that starts with an uppercase

lennondtps commented 1 year ago

@kpacha Thanks for your feedback!

Do you mean something like this?

type producerCfg struct {
    queueCfg
    Mandatory        bool   `json:"mandatory"`
    Immediate        bool   `json:"immediate"`
    ExpirationKey    string `json:"exp_key"`
    ReplyToKey       string `json:"reply_to_key"`
    MessageIdKey     string `json:"msg_id_key"`
    PriorityKey      string `json:"priority_key"`
    RoutingKey       string `json:"routing_key"`
    StaticRoutingKey bool   `json:"static_routing_key"`
}

Have a boolean parameter to define whether the routing_key value is static? Let me know if you have something different in your mind, and I will implement it!

kpacha commented 1 year ago

@lennondtps yes, that's exactly what I had in mind. I think that could offer the complete feature.

lennondtps commented 1 year ago

Hello @kpacha, I also opened a PR here, in order to include the new attribute in the documentation https://github.com/krakend/krakend-schema/pull/24/files

lennondtps commented 1 year ago

@alombarte @kpacha This will be merged in master or you will create another branch?

alombarte commented 1 year ago

Hi @lennondtps , this will be merged into master and released with KrakenD 2.5 which is out within this quarter