Nservicebus: Remove double opt-in for the Outbox

[timbussmann]

The Outbox currently requires a double opt-in mechanism via code & config file to be enabled. As raised in the discussion here https://github.com/Particular/NServiceBus/issues/3275 it doesn't seem to provide much value and may behave unexpectedly.

Is there any good reason why we should keep the double opt-in mechanism?

[andreasohlund]

Is this really a breaking change?

[bording]

If we're just talking about removing the code that looks for the appsetting, then it seems like it wouldn't be breaking. Anyone already using it would have a leftover setting their config file they could remove.

Since the existing code throws an exception when you have it enabled in code but not config, it shouldn't be possible to have anyone suddenly finding the Outbox enabled when they didn't have it before either.

[timbussmann]

Since the existing code throws an exception when you have it enabled in code but not config, it shouldn't be possible to have anyone suddenly finding the Outbox enabled when they didn't have it before either.

that's a good point. But what about the other way round? Enable it by default in the config and have some logic whether you're using the code based API?

[andreasohlund]

I guess that would be breaking technically so I'm good with keeping this flagged as breaking.

On 31 Oct 2016, at 09:28, Tim Bussmann [email protected] wrote:

Since the existing code throws an exception when you have it enabled in code but not config, it shouldn't be possible to have anyone suddenly finding the Outbox enabled when they didn't have it before either.

that's a good point. But what about the other way round? Enable it by default in the config and have some logic whether you're using the code based API?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[timbussmann]

talking to @bording my comment doesn't really hold up since the only valid use case would be that the appsetting is set to true. in that case the & logic is redundant since enabled & true = enable, disable & true = disable. So that would be no breaking change again. What do you think @andreasohlund ?

maybe @bording can verify that we throw an exception when the setting is not around or false but enabled in code.

[andreasohlund]

We do throw if the config value is null/"false"

https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Reliability/Outbox/Outbox.cs#L34

[andreasohlund]

So in that case I'd say this isn't a breaking change. Label removed

[bording]

Looks like it only throws if you've called EnableOutbox and don't have the appsetting defined at all in your config file. Since that's not a valid combination, we can ignore this for the "is this breaking?" question.

If you've added the appsetting and set it to true, then this also wouldn't be a behavior break, because the Outbox would be enabled solely based on your code, which will already either be calling EnableOutbox or not.

If you've added the appsetting and set it to false, then you could potentially see a behavior break, because if you're calling EnableOutbox, you wouldn't actually be enabling the Outbox. This means that us removing the double opt-in could mean you'd find yourself with the Outbox suddenly enabled.

However, I'd argue that this case is actually a bug, because right now it silently fails to enable the Outbox without informing you in any way. By removing the double opt-in, we'd be respecting the behavior that the user is expecting.

At least is in V5, starting the endpoint would log that the Outbox feature was disabled:

Name: Outbox
Version: 5.2.19
Enabled by Default: No
Status: Disabled
Deactivation reason: Did not fulfill its Prerequisites:
   -This transport requires outbox consent

But we don't do this in V6.

[Sweetog]

The Double-Opt In is inconvenient and unnecessary, it would be great if we could remove the config requirement, if someone is going to EnableOutbox they will have taken the time to understand the ramifications, it is not a confirmation to make someone add the value in a config file as well

[timbussmann]

if someone is going to EnableOutbox they will have taken the time to understand the ramifications, it is not a confirmation to make someone add the value in a config file as well

thanks for chiming in @Sweetog 👍 I fully agree with your statement.

[danielmarbach]

@Particular/nservicebus-maintainers thoughts?

https://github.com/Particular/NServiceBus/pull/4440