Support connection name

[brerx]

related to #599.

• adds connection_name as optional client_property
• adds unit test

[brerx]

@michaelklishin This is my first approach to solve this.

Without the addition in docker-entrypoint, the CLI tool proceeded before the actual users were created, which resulted in countless Auth errors during testing. So this should enable more people to contribute here.

I am eager to hear your feedback, especially whether the unit test is in the right place. I was not sure whether to put it rather into the integration/connection_spec...

[michaelklishin]

It makes no sense to sleep before rabbitmqctl await_startup. That command waits for node boot to complete. User creation is synchronous, so I don't see any need to wait after we set up the permissions but at least it would make some sense there.

[brerx]

Ok, as an unrelated change, I'll remove this again. No problem.

[michaelklishin]

Yes, we can address this separately if needed. Admittedly I do not use this image for testing as I usually test Bunny against RabbitMQ nodes built from source or GA releases running locally.

[michaelklishin]

Hash#fetch makes sense when you want a non-nil default. Should we use Hash#[] here?

[michaelklishin]

This does not actually populate the client property by merging it into @client_properties. So this makes it easy to access connection name in the app but not set it, which I thought was the goal?

[brerx]

Ah now I think I know how you want to understand it: Instead of

Bunny.new(properties: { connection_name: 'name' } }

which will merge automatically to the @client_properties, you thought of something like this:

      @client_properties   = DEFAULT_CLIENT_PROPERTIES.merge(client_props).merge(connection_name: opts[:connection_name])

=> Bunny.new(connection_name: 'name')

Am I right? I like this even better.

[michaelklishin]

Correct, it should be easy to set a connection name, reading it is primarily useful for tests and, perhaps, logging.

Bunny.new(properties: { connection_name: 'name' } } would still work. I'm not sure whether connection_name the option or client_properties should take precedence, your approach seems right.

[brerx]

Ok, I changed it to the easier usecase. What do you think?

[michaelklishin]

I have updated the code to support connection names specified directly in client_properties. Will add a spec example next. The rest looks good to me.

[brerx]

Thx for merging this so fastly. Will you make a new release soon?

[michaelklishin]

2.17.0 is up on rubygems.org.