Nservicebus: Make selection of serializer mandatory

[andreasohlund]

The current version defaults to XML, I propose we make the selection mandatory to solve the following problems.

• The azure transports in v5 and below changes the default to JSON since that's more appropriate due to size constraints of those transports. There is no way to flip defaults in the upcoming version and I think its good to educate users and have them make a conscious selection based on factors like:
  
  
  
  • Size
  
  
  • Native integration needs
  
  
  • Message versioning (ie do not use binary serialization, ever)
  
  
• This would also allow use to get rid of the json.net serializer in the core since Azure would no longer default to it
  
  
  
  • We can guide the users via obsolete messages to the external version we already have.
  
  

I propose we include this in v6 since it would avoid providing a backdoor that azure can use and its IMO the most future proof solution.

[timbussmann]

There is no way to flip defaults in the upcoming version

:+1:

As mentioned, my only concerns about this approach is that it raises the serializer on the same level as the transport or persistence. But while users need to define transport & persistence because they must provide the infrastructure for it, that's not necessary for serializer. Users are able to run a system using NSB without knowing about the used serializer. IdK if that's alright or not.

I think Azure defaulting to JSON is wrong (while the idea behind it is perfectly reasonable), it should stick to the default like every other transport. We currently "hide" the choice for the default serializer which is probably the dangerous part. Even if a user knows that he should use JSON for Azure, he may not realize that we use XML instead. I guess the only way to make users aware of this is to have them select their option.

[yvesgoeleven]

Azure transports were defaulting to Json in the past, as the maximum message size is rather small (64K for ASQ & 256K for ASB), you hit that quickly with xml

I agree with @timbussmann that defaulting in general is wrong, we shouldn't hide this aspect from the user. Can we still include this in v6?

[andreasohlund]

So we need to decide on:

• Make serializer selection mandatory

OR

• Hack ASB and ASQ to flip the default to JSON just like v5

Is it even possible to do the no.2 hack?

How much effort does each option take?

[timbussmann]

what about just throwing an exception when a different than JSON serializer is defined (without some explicit confirmation) when using Azure? This will force Azure users to define the serializer ("mandatory selection") while not changing the behavior for others?

every mandatory selection hurts the first time experience that's why I dislike the "mandatory serializer selection by default".

[yvesgoeleven]

@timbussmann json is not mandatory, it's just the most logical choice. So throwing is not an option.

[andreasohlund]

Talking to @timbussmann (and partly with @yvesgoeleven ) we agreed that while mandatory serializer selection is a good idea we need to improve our story for bootstrapping new users before adding more mandatory configuration options.

Also the ideas around a Container/Compartment/Endpoint model will probably affect the way we force the users to specify mandatory options so delaying this would allow us to get more input in this area as well.

We propose to keep the current v5 behavior for v6 and that would require the following:

1. Modify the core to allow default serializer selection to be changed
2. Modify ASB to change the default to json just like in v5
3. Modify ASQ to change the default to json just like in v5

Since we have to take action in order to not change serializer behind the back of existing ASB/ASQ v5 users I will flag the WIP pull as core v6 once we agree.

@Particular/tf-v6-launch-plat-dev @Particular/azure-maintainers @Particular/nservicebus-maintainers thoughts?

[Scooletz]

If we make the default serializer changeable from the transport level, we'll need to remove the code related to the handling of serialization in the ASQ and just use this option: https://github.com/Particular/NServiceBus.AzureStorageQueues/blob/develop/src/Transport/MessageWrapperSerializer.cs

[SeanFeldman]

:+1: for this change. I'd rather default to JSON than have a default that makes no sense (XML) if there's no option to enforce selection of serializer.

[SimonCropp]

My vote is to force selection of serializer are config time.

[indualagarsamy]

Making the user select the serializer at configuration time removes all ambiguity from having to choose different sensible defaults for different transports and maintaining what that list is. Simplifies the code. For MSMQ customers, this would be adding an extra line to add the XmlSerializer at configuration time and they'd be running as before.

[andreasohlund]

I'm on the fence, me and @timbussmann came to the conclusion that we want to make out first time user story better (bootstrap packages, devtransport, devpersistence, launchpad website) before we make it mandatory. The api for specifying the serializer will most likely change when we do https://github.com/Particular/PlatformDevelopment/issues/621

Based on this we decided to hold of with this until v7 since we hope to be in a better position then?

[danielmarbach]

I'd rather go with the more explicit option, which means the user has to select the serializer. Clearer, less surprises and unwanted behavior when you change the transport. It only makes it slightly worse. When we move to https://github.com/Particular/PlatformDevelopment/issues/621 we need to come up with an API anyway which will have mandatory (but hopefully strongly typed) settings that need to be passed in. The argument @andreasohlund is for me not a NOGO to not enforce the serializer.

[yvesgoeleven]

I was also in favor of just making the choice mandatory. I'm not so concerned about the impact on first time experience in a sense that it won't make the experience crappier than it already is for azure users as they already have to switch transport, persistence, configure error queue etc... The config is visible in your intellisense when you choose persistence and transport, so it's not that hard to set it.

[indualagarsamy]

+1 to what @yvesgoeleven said.
We are already forcing the user to pick the persistence, which is mandatory. So, it won't be a surprise to add serialization to that list. How about we make the transport choice mandatory too? So for MSMQ users, they'd have to add two additional lines to configuration, i.e. on transport and serialization.
This will also facilitate removing MSMQ into its own separate transport at some time.

I'd argue that making the persistence choice mandatory in Version 5 was a much bigger change than what's proposed now for v6.

[boblangley]

:+1: Mandatory all the things

On Thu, Mar 24, 2016, 12:57 PM Indu Alagarsamy [email protected]
wrote:

+1 to what @yvesgoeleven https://github.com/yvesgoeleven.
We are already forcing the user to pick the persistence, which is
mandatory. So, it won't be a surprise to add serialization to that list.
How about we make the transport choice mandatory too? So for MSMQ users,
they'd have to add two additional lines to configuration, i.e. on transport
and serialization.
This will also facilitate removing MSMQ into its own separate transport at
some time.

I'd argue that making the persistence choice mandatory in Version 5 was a
much bigger change than what's proposed now for v6.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly or view it on GitHub
https://github.com/Particular/NServiceBus/issues/3436#issuecomment-200993126

[timbussmann]

I'm still :-1: for making serializer selection mandatory.

while users need to define transport & persistence because they must provide the infrastructure for it, that's not necessary for serializer. Users are able to run a system using NSB without knowing about the used serializer

I think this is a very important part of the first time experience (which we want to improve). So making that experience even worse "because it's already bad" is not an argument I agree with. Especially with the current API which basically throws at startup. That's so bad that I think we need to prevent this under any circumstance. I think I could live with making it mandatory in a "type safe" way (like endpoint name) so we at least can avoid runtime exceptions but even then I'm not really happy about that.

[andreasohlund]

I'm still with @timbussmann , I don't see this as a must have for v6. I'd rather make the transport mandatory instead to pave the way for msmq out of the core to get us dotnetcore ready

[andreasohlund]

Closing this, we're having internal discussions on if/how to evolve our serializer selection in the future

[SimonCropp]

closing this, we're having internal discussions

I have noticed several of these in recent months. Isnt this the antithesis of the open source ethos. I know when Microsoft does this they get criticised for it and often come to the wrong decision since that decision is come to in a bubble and lacks many real world perspectives

[andreasohlund]

Reopening this since we haven't reached a conclusion yet. Thanks for calling this out @SimonCropp