-
Notifications
You must be signed in to change notification settings - Fork 36
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
Support multi-instance configuration as a legacy feature #161
Conversation
This one is can continue when #162 is ready |
6e75a7a
to
6d86643
Compare
@justabitofcode please review. I've moved code into separate classes because otherwise sql connection management in normal mode of operation and in legacy multi-instance mode made |
{ | ||
c.UseTransport<SqlServerTransport>() | ||
.UseSpecificSchema(queueName => queueName.Contains("Receiver") ? "receiver" : "sender") | ||
.EnableLagacyMultiInstanceMode(async address => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think calling it "Legacy" gives the impression that you don't need to do anything. This is more like backwards compatibility, but handled in a new way. Would calling it "MultiInstanceCompatMode" or something like that be more precise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why "Legacy" has been used is to indicate that we users should avoid it as there are better ways to do it and that in v4 we will not support it any more. I'm not sure "Legacy" is the best word here though :/.
OK, no more comments, but @justabitofcode should have a close look too. |
@@ -82,5 +82,20 @@ public static TransportExtensions<SqlServerTransport> TransactionScopeOptions(th | |||
transportExtensions.GetSettings().Set<SqlScopeOptions>(new SqlScopeOptions(timeout, isolationLevel)); | |||
return transportExtensions; | |||
} | |||
|
|||
/// <summary> | |||
/// Specifies connectionString lookup to be used by sql transport. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have an extra line of summary to indicate that this is for backwards compatibility and point customers to the new version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I've obsoleted the extension point and extended description.
a7db109
to
4206da4
Compare
Support multi-instance configuration as a legacy feature
Those are changes based on @justabitofcode multi-instance spike. I did following things:
legacy
mode of operationLegacy.MultiInstance
folderPoA:
Connects to: #139