-
Notifications
You must be signed in to change notification settings - Fork 17
[WIP] Fixed ToTransportAddress by making the translation explicit #316
Conversation
… included properties of address).
@SzymonPobiega it doesn't appear to compile on the buildserver due to missing QueueName & Discriminator properties, is there some version dependency mismatch? |
@yvesgoeleven it is WIP because the parent one is not yet merged. I'll create a non-WIP version that does not depend on that change in the core. |
@@ -27,7 +27,19 @@ public override EndpointInstance BindToLocalEndpoint(EndpointInstance instance) | |||
|
|||
public override string ToTransportAddress(LogicalAddress logicalAddress) | |||
{ | |||
return logicalAddress.ToString(); | |||
var queue = new StringBuilder(logicalAddress.QueueName); |
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.
Why couldn't this be encapsulated in LogicalAddress.ToString()
?
Or, alternatively, not to have a LogicalAddress.ToToTransportAddress()
?
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.
Because the actual code depends on the transport. I mean, different transport can use different separators for qualifiers and discriminators (e.g. SQL) and can use additional properties (MSMQ, SQL)
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.
Thank you @SzymonPobiega.
So discriminator and qualifier are mutually exclusive? Sorry for pestering with these questions, want to understand :)
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.
Yes, in the current core code qualifier and discriminator are mutually exclusive. There is even a check in LogicalAddress
which prevents creating an address with both. The idea is that the satellite queues (which have qualifier) are always shared by all instances.
I am not entirely sure if this is a permanent property but it seems unlikely that we'll have a per-instance satellites any time soon.
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.
Thank you @SzymonPobiega
Replaced by #318 which does not require changes in the core. |
The previous version with
ToString
would include all instance properties (if provided) which is not something we want in the address.