dashbitco/broadway_rabbitmq

AMQP 2.0

Closed this issue · 3 comments

ono commented

Hi there. I am a co-maintainer of amqp library. I am currently preparing a major version release for the library.

Since broadway_rabbitmq seems to be the most popular library that uses amqp, I'd like to get your feedback before releasing the 2.0.0.

As you can see the release notes, there are couple breaking changes.

no_wait to nowait

amqp still supports no_wait option so you don't have to make any changes for this.

Connection name

Connection name is now moved to options parameter. You need to make a little change on BroadwayRabbitMQ.AmqpClient.setup_channel/1.

Since the option was introduced at 1.5 you will also need to change the dependency requirement like this:

{:amqp, "~> 1.5 or ~> 2.0"},

If this can block you supporting 2.0, I can consider dropping the change on Connection.open and keep supporting connection name as a separate parameter. What do you think?

If you have any other questions or/and feedback, please let me know.

Thanks!

@ono we're good with switching to the :name being an option instead of a parameter. In AMQP 2.0, will we have Connection.open/2 at all? I’m trying to figure out what is the best way of checking whether we should pass the name as an option or as a parameter.

ono commented

AMQP 2.0 will support Connection.open/2 but it takes only connection string(binary) as a first parameter and options (keyword) as a second parameter.

You can replace the function call with a private function like below:

defp open_connection(url, name) when is_binary(url) do
  Connection.open(url, name: name)
end

defp open_connection(opts, name) when is_keyword(opts) do
  Connection.open(opts ++ [name: name])
end

defp open_connection(nil, name) do
  Connection.open(name: name)
end

Then you can use open_connection(config.connection, name) instead.

However, to be honest, I am a bit inclined to drop the Connection.open change from 2.0 update. The main change I'd like to introduce is SelectiveConsumer. Seeing the name option is used widely like here, I feel we shouldn't keep the backward compatibility.

Please leave it to me for a moment. I will come back to you tonight or tomorrow.

ono commented

Hi Andrea,

We are going to drop Connection.open change from 2.0 release. It's not the important change at this timing or much hassle to maintain the current format along with a new format. I will open another PR or issue after 2.0 release is done.

Thanks!