Only open the admin connection when we need to use it

[andreasohlund]

Endpoints keeps the admin connection (to perform sub/unsubscribe) open. This is wasteful since all subscribes usually happens on start up.

[johnsimons]

Looks good

On Sunday, January 11, 2015, Andreas Öhlund notifications@github.com
wrote:

@johnsimons https://github.com/johnsimons I noticed that all endpoints
keep the admin connection (to perform sub/unsubscribe) open. This seems
wasteful since all subscribes usually happens on start up.

What do you think?

You can merge this Pull Request by running

git pull https://github.com/Particular/NServiceBus.RabbitMQ open-admin-connection-on-demand

Or view, comment on, or merge it at:

#71
Commit Summary

• Only open the admin connection when we need to use it

File Changes

• M src/NServiceBus.RabbitMQ/RabbitMqConnectionManager.cs
  https://github.com/Particular/NServiceBus.RabbitMQ/pull/71/files#diff-0
  (7)
• M src/NServiceBus.RabbitMQ/RabbitMqSubscriptionManager.cs
  https://github.com/Particular/NServiceBus.RabbitMQ/pull/71/files#diff-1
  (6)

Patch Links:

• https://github.com/Particular/NServiceBus.RabbitMQ/pull/71.patch
• https://github.com/Particular/NServiceBus.RabbitMQ/pull/71.diff

—
Reply to this email directly or view it on GitHub
#71.

[andreasohlund]

The issue is that we're now mixing the responsibility for disposing the
connection. Lets talk it through in the bar in Cyprus

On Sun, Jan 11, 2015 at 12:30 AM, John Simons notifications@github.com
wrote:

Looks good

On Sunday, January 11, 2015, Andreas Öhlund notifications@github.com
wrote:

@johnsimons https://github.com/johnsimons I noticed that all
endpoints
keep the admin connection (to perform sub/unsubscribe) open. This seems
wasteful since all subscribes usually happens on start up.

What do you think?

You can merge this Pull Request by running

git pull https://github.com/Particular/NServiceBus.RabbitMQ
open-admin-connection-on-demand

Or view, comment on, or merge it at:

#71
Commit Summary

• Only open the admin connection when we need to use it

File Changes

• M src/NServiceBus.RabbitMQ/RabbitMqConnectionManager.cs
  https://github.com/Particular/NServiceBus.RabbitMQ/pull/71/files#diff-0

(7)

• M src/NServiceBus.RabbitMQ/RabbitMqSubscriptionManager.cs
  https://github.com/Particular/NServiceBus.RabbitMQ/pull/71/files#diff-1

(6)

Patch Links:

• https://github.com/Particular/NServiceBus.RabbitMQ/pull/71.patch
• https://github.com/Particular/NServiceBus.RabbitMQ/pull/71.diff

—
Reply to this email directly or view it on GitHub
#71.

—
Reply to this email directly or view it on GitHub
#71 (comment)
.

[andreasohlund]

@johnsimons @SimonCropp while I think the fix is good (no need to keep the admin channel open) I have a bad feeling about the RabbitMqConnectionManager class. It seems wrong that some calls returns a connection that is managed by the connection manager (publish + consume) and one (admin) that is supposed to be managed by the caller. Do U have any idea on how to clean this up?

[johnsimons]

@andreasohlund how about making the API more explicit?

• GetPublishConnection rename to GetOrCreatePublishConnection
• GetConsumeConnection rename to GetOrCreateConsumeConnection
• GetAdministrationConnection rename to CreateAdministrationConnection

[andreasohlund]

Yes that is better. Since it would be a breaking change I'll raise a new
issue to track it and slate if for v 3.0.0

On Fri, Feb 6, 2015 at 6:09 AM, John Simons notifications@github.com
wrote:

@andreasohlund https://github.com/andreasohlund how about making the
API more explicit?

• GetPublishConnection rename to GetOrCreatePublishConnection
• GetConsumeConnection rename to GetOrCreateConsumeConnection
• GetAdministrationConnection rename to CreateAdministrationConnection

—
Reply to this email directly or view it on GitHub
#71 (comment)
.