Transports should not access core settings via the settings holder

[andreasohlund]

Instead we should pass the needed settings in the transport seam.

This issue will start with surveying the use in downstreams in order to serve as input to a "transport seam vNext".

SqlTransport

• Figuring out if the TM is enabled or not settings.GetOrDefault<FeatureState>(typeof(TimeoutManager).FullName) == FeatureState.Disabled;
• The local address in order to process delayed messages var delayedMessageProcessor = new DelayedMessageProcessor(dispatcher, settings.LocalAddress());
• Logical address in order to create the delayed messags queue: var delayedQueueLogialAddress = settings.LogicalAddress().CreateQualifiedAddress(delayedDeliverySettings.Suffix);
• Append schema and catalog info to endpoint instances: settings.GetOrCreate<EndpointInstances>().AddOrReplaceInstances("SqlServer", schemaAndCatalogSettings.ToEndpointInstances());
• Native delayed delivery checks for external timeout manager and send-only endpoints: var externalTimeoutManagerAddress = settings.GetOrDefault<string>("NServiceBus.ExternalTimeoutManagerAddress") != null; var sendOnlyEndpoint = settings.GetOrDefault<bool>("Endpoint.SendOnly");

Learning

• Error queue address: var errorQueueAddress = settings.ErrorQueueAddress();
• Endpoint name for native pub sub
• Local address for native pub sub

MSMQ

• Send only : settings.GetOrDefault<bool>("Endpoint.SendOnly")
• If the error q has been explicitly set in core settings.TryGetExplicitlyConfiguredErrorQueueAddress(out string _))
• Queue bindings var bindings = settings.Get<QueueBindings>()
• Transaction mode: var isTransactional = settings.GetRequiredTransactionModeForReceives() != TransportTransactionMode.None;
  • Is outbox on : settings.IsFeatureActive(typeof(Outbox));
• Audit message expiration: settings.TryGetAuditMessageExpiration(out var auditMessageExpiration);

ASQ (Not complete)

• GetRequiredTransactionModeForReceives() to figure out if to run in atmost once or at least once mode

ASB

TBD

Rabbit

TBD

SQS

TBD

EventStore

TBD

NSB Raw

Needs to provide all relevant settings to mimic what the core does which is very error prone and brittle

[andreasohlund]

@Particular/sqlserver-transport-maintainers can you double check that I got the section about your transport right?

[MarcinHoppe]

@andreasohlund I have no context: what problem are we solving here?

[andreasohlund]

We have many cases of transports ”pulling stuff out of settings” and that leads to bugs due to invalid assumptions, timing etc In the future we want the core to pass relevant settings explicitly in the transport seam so that transports would only user ”settings” for their own transport specific dials and knobs.

This issue just captures the current lay of the land.

Does this make sense?

[MarcinHoppe]

@andreasohlund I added two more findings for the SQL Server transport in the issue description.

[andreasohlund]

Added

NSB Raw

Needs to provide all relevant settings to mimic what the core does which is very error prone and brittle

[timbussmann]

👍 but I think this needs to be raised on pdev.

[andreasohlund]

Yea, will raise a more overarching issue in pdev soon On Tue, 7 Nov 2017 at 10:42, Tim Bussmann notifications@github.com wrote:

👍 but I think this needs to be raised on pdev.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Particular/NServiceBus/issues/5044#issuecomment-342428339, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHoZBN--Ztuh1RSX5Csw2iasREnQfEoks5s0CYdgaJpZM4QCaBF .

[bording]

One category of items that will be common to a lot of transports is all of the stuff needed to make the timeout manager "hybrid" mode work for backcompat for native delays.

Adding support for that in core directly would eliminate a lot of stuff from the transports.