Nservicebus: Better API for Outbox settings

[weralabaj]

When reviewing docs for Outbox recently, we noticed that 1. Raven's and NHibernate's APIs are not aligned, 2. the API could be easily done better.

@mauroservienti proposed the following option:

configuration.EnableOutbox() .SetTimeToKeepDeduplicationData( ... ) .SetFrequencyToRunDeduplicationDataCleanup( ... );

Both me and @SzymonPobiega think it's a good proposition - it'll be easier to use and will guide users in the right direction.

Additionally, that also looks like worth considering:

configuration.EnableOutbox<RavenDB.../NHibernate...>()

Context:
See Raven API

<appSettings> <add key="NServiceBus/Outbox/NHibernate/TimeToKeepDeduplicationData" value="7.00:00:00" /> <add key="NServiceBus/Outbox/NHibernate/FrequencyToRunDeduplicationDataCleanup" value="00:01:00" /> </appSettings>

See NHibernate API

endpointConfiguration.SetTimeToKeepDeduplicationData(TimeSpan.FromDays(7)); endpointConfiguration.SetFrequencyToRunDeduplicationDataCleanup(TimeSpan.FromMinutes(1));

Related issues:
https://github.com/Particular/NServiceBus.RavenDB/issues/262
https://github.com/Particular/NServiceBus.NHibernate/issues/207

Ideally, we'd like to tackle it as part of RC.

[SzymonPobiega]

@weralabaj it's worth noticing that from the NH and Raven repos, this is a maintainer prio thing.

I think we can investigate both solutions.

configuration.EnableOutbox<MyOutboxPersistence>()
.SetTimeToKeepDeduplicationData( ... ) 
.SetFrequencyToRunDeduplicationDataCleanup( ... )

would be cleaner because the actual setter methods would be defined in the persister, not in the core. The alternative to put these two methods in the core would probably be easier but I think it is worth PoC-ing the first variant first.

[andreasohlund]

Would it be possible to make this change in a 6.1 and obsolete the current api with a WARN or is this a thing that would need to be synchronized with both Raven and NH?

[weralabaj]

It depends on what option we decide. But given it's not a big thing, wouldn't it be worth doing now?
Especially for NHibernate, because we don't have code API now :/

[andreasohlund]

Not sure, we have plenty of api's that needs some love but we need to draw the line somewhere.

Is the non breaking version suboptimal compared to the breaking one?

[weralabaj]

we need to draw the line somewhere
Is the non breaking version suboptimal compared to the breaking one?

I have a feeling that we're moving off-topic. I don't have any input on that subject, sorry, but I don't have enough context to comment. If you say "no changes in core" then we can go with the extensions methods in persistence only, seems like OutboxSettings give us access to everything we need.

Is there anything in the description and Szymon's comment that's not clear or is something missing so you can decide about the core part?

[andreasohlund]

Seems like we can do it without touching the core (can you confirm @SzymonPobiega )

So that means we can close this and open relevant issue in NH+Raven?

I guess the argument still holds that this can be done in a minor version of the persisters so there is no dependency on v6 => In that case I'd leave it up to the maintainers of NH + RavenDB to deal with it appropriately.

Does this make sense?

[weralabaj]

The issues are already opened:

Related issues:
Particular/NServiceBus.RavenDB#262
Particular/NServiceBus.NHibernate#207

If we don't want to make configuration.EnableOutbox() => configuration.EnableOutbox<RavenDB.../NHibernate...>() transition then yes, we can stick to persistence changes only, for example by implementing extension methods on OutboxSettings in NHibernate and Raven.

[andreasohlund]

That seems good enough to me? Ok to remove the v6 milestone?

[danielmarbach]

Based on @weralabaj comment I'm closing this since we decided to address those API changes in the persisters.