Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt-out from reusing SQL Server transport's connection #148

Merged
merged 2 commits into from
Aug 25, 2017

Conversation

SzymonPobiega
Copy link
Member

Adds DoNotUseSqlServerTransportConnection to the SQL Server dialect settings and moves the code responsible for adapting the transport connection to the dialects.

Fixes #144

}

/// <summary>
/// Instructs the persistence to not use the connection established by the SQL Server transport.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking how weird this is going to look to someone who isn't also using the SQL Transport. The comment here needs to tell the user why this would be necessary, and the answer is only if you were using a different connection string between SQL Transport and SQL Persistence, right?

So that got me thinking - can't we just look at the connection strings and enlist only if they match?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidBoike that used to be an option with NHibernate but since SQL persistence uses Func<DbConnection> it would mean that for each message we need to create the persistence connection using the factory and check if connection string's match.

Alternatively we can assume that the connection factory always returns connection with the same connection string and check only once but I am not sure if we can make such an assumption.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we also can't detect when they have it misconfigured and throw an appropriate exception, because we don't even try creating a connection until we've determined we can't enlist, right?

I'm not sure I like that very much. I wouldn't want to wait around for an "invalid object name" exception with no further clue to what's really going on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about trying to create the connection on strartup to capture the connection string there? We do that in SQL transport if user provides a func:

https://github.com/Particular/NServiceBus.SqlServer/blob/develop/src/NServiceBus.SqlServer/SqlServerTransport.cs#L64

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SzymonPobiega I like the connection string capture on startup idea.

@SzymonPobiega
Copy link
Member Author

@Particular/sqlpersistence-maintainers ping?

@SimonCropp SimonCropp merged commit b03f61a into develop Aug 25, 2017
@SimonCropp SimonCropp deleted the add-option-to-not-use-transport-connection branch August 25, 2017 21:00
@SzymonPobiega SzymonPobiega added this to the 3.0.0 milestone Nov 22, 2017
@SzymonPobiega SzymonPobiega changed the title Add option to not use transport connection Opt-out from reusing SQL Server transport's connection Nov 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants