wagslane / go-rabbitmq

A wrapper of streadway/amqp that provides reconnection logic and sane defaults
https://blog.boot.dev/golang/connecting-to-rabbitmq-in-golang-easy/
MIT License
772 stars 126 forks source link

Why is Exchange configuration driven by Consumers instead of Publishers #43

Closed ghost closed 1 year ago

ghost commented 2 years ago

This library seems to have Consumers drive the configuration of the Exchanges rather than the Publishers. For example, Consumers determine if an exchange should auto delete, be durable, or the kind (e.g. topic).

Sample:

    err = consumer.StartConsuming(
            func(d rmq.Delivery) rmq.Action {
                log.Printf("consumed: %v", string(d.Body))
                // true to ACK, false to NACK
                return rmq.Ack
            },
            QUEUE,
            []string{TOPIC},
            rmq.WithConsumeOptionsBindingExchangeAutoDelete,
            rmq.WithConsumeOptionsBindingExchangeDurable,
            rmq.WithConsumeOptionsBindingExchangeName(""),
            rmq.WithConsumeOptionsBindingExchangeKind("topic"),
            rmq.WithConsumeOptionsQueueArgs(queueArgs),

Isn't this a bit backwards since Publishers are the ones who write to Exchanges and should therefore configure its settings, and then Consumers simply configure the queues that they use? Was this design selected out of personal preference or was there a technical reason for it?

wagslane commented 2 years ago

It's a good question, and I'm open to changing my mind, that said let me present my reasoning.

Primarily, this library is a "batteries included" library, the goal is sane defaults and such. It makes sense to me that you should be able to start a consumer and have it "just work", even if the exchange you're attaching to doesn't yet exist. If it doesn't your consumer will create it for you. That said, the default options WILL NOT create an exchange for you, you have to ask for that explicitly.

Additionally, this library will error out and alert you if you try to create an exchange that already exists and it doesn't have the parameters you expect:

ExchangeDeclare declares an exchange on the server. If the exchange does not already exist, the server will create it. If the exchange exists, the server verifies that it is of the provided type, durability and auto-delete flags.

ghost commented 2 years ago

It's a good question, and I'm open to changing my mind, that said let me present my reasoning.

Primarily, this library is a "batteries included" library, the goal is sane defaults and such. It makes sense to me that you should be able to start a consumer and have it "just work", even if the exchange you're attaching to doesn't yet exist. If it doesn't your consumer will create it for you. That said, the default options WILL NOT create an exchange for you, you have to ask for that explicitly.

Additionally, this library will error out and alert you if you try to create an exchange that already exists and it doesn't have the parameters you expect:

ExchangeDeclare declares an exchange on the server. If the exchange does not already exist, the server will create it. If the exchange exists, the server verifies that it is of the provided type, durability and auto-delete flags.

understood, thanks for the explanation! logical and reasonable

i ended up forking the repository in order to have publisher-driven exchange creation and consumer-driven queue creation

phillipj commented 2 years ago

@wagslane what's your thoughts on also allowing publishers to create the exchange, not only the consumer?

The current behaviour caught me as a surprise. Having gone through the streadway/amqp examples, my brain was wired into the idea that both parties could create the exchange. Combined with the idea of go-rabbitmq handling the low-levels for us, I for some reason struggled badly understanding why on earth the exchange was not created when the publisher interacting with RabbitMQ.

Honestly didn't even think it might have helped to start a consumer at that point, we didn't have that service ready yet, but wanted to rollout the publishing part out first and verify the RabbitMQ mechanics were created as expected.

Steering away from a philosophical discussion about right or wrong, if we had the opportunity to choose if the exchange could be created by the publisher too, that would have been really helpful for our workflow & desire for independent rollouts.

NicklasWallgren commented 2 years ago

It would be really helpful if the publisher could create the exchange as well.

wagslane commented 2 years ago

I'm totally open to that idea. Feel free to open a PR or I'll get to it when I can

NicklasWallgren commented 2 years ago

Great, I might have a go at it later this week.

NicklasWallgren commented 2 years ago

I've started working on a PR in my local fork, if anyone is interested.

https://github.com/NicklasWallgren/go-rabbitmq/pull/1/files

aaqaishtyaq commented 1 year ago

@h44z @wagslane If the design is finalized, Can I help?

Having configuration driven by either Publisher or Consumer would be good to have. Otherwise, One has to write the consumer first before publishing anything to the rabbitmq.

NicklasWallgren commented 1 year ago

@aaqaishtyaq Have a look at https://github.com/wagslane/go-rabbitmq/pull/91

aaqaishtyaq commented 1 year ago

@NicklasWallgren Thanks for the PR. Can we please look into merging it?

mijjjj commented 1 year ago

When considering merging this pr, our service environment consumers will restart the service, which will cause our publishers to be unable to publish messages

wagslane commented 1 year ago

Everyone concerned with this thread please read the new docs on the main branch. This issue should be resolved. Please let me know as soon as you can if this new API will work.

wagslane commented 1 year ago

Resolved in 0.11.0