Particular / NServiceBus

Build, version, and monitor better microservices with the most powerful service platform for .NET
https://particular.net/nservicebus/
Other
2.1k stars 647 forks source link

Does Auditing need its own configuration section, rather than using UnicastBusConfig? #1339

Closed indualagarsamy closed 11 years ago

indualagarsamy commented 11 years ago

Why is Auditing configuration stored in UnicastBusConfig?

The two properties, ForwardReceivedMessagesTo and TimeToBeReceivedOnForwardedMessages are currently stored in UnicastBusConfig.

We are currently planning to move Audit as its own feature, related GH #1309 Should we create MessageAuditingConfig and add these two properties to this new config section and obsolete the ones in UnicastBusConfig?

indualagarsamy commented 11 years ago

@udidahan @andreasohlund Thoughts on this?

andreasohlund commented 11 years ago

Moving the config seems resonable to me

Sent from my iPhone

On 24 jul 2013, at 07:54, Indu Alagarsamy notifications@github.com wrote:

@udidahan @andreasohlund Thoughts on this?

— Reply to this email directly or view it on GitHub.

udidahan commented 11 years ago

In which case, shouldn't we also look at replacing UnicastBusConfig with something whose name is more clear - like RoutingConfig?

andreasohlund commented 11 years ago

Yes that makes sense. The .UnicastBus() call will also go away as we refactor the fluent api.

andreasohlund commented 11 years ago

So: RoutingConfig and AuditConfig replaces the UnicastBusConfig

We support both in v5 but with a warning logged if they use the old version(like we did with the msmqtransportconfig)

Makes sense?

andreasohlund commented 11 years ago

Where do we put the Distributor (control + data) setting?

ScaleOutConfig , DistributorConfig?

indualagarsamy commented 11 years ago

Currently, we have MasterNodeConfig which is separate. However, it applies for the Distributor feature. Should we have a config section called, MsmqScaleOutConfig? (since this is for Msmq only?) for all the distributor stuff?

SimonCropp commented 11 years ago

Ok I am going to play the devils advocate here.

I think we should approach "adding new config sections" with caution.

They take a large amount of code to build, are very rigid and I just dont see the value proposition over using appsettings.

Look at simple values

<configSections>
  <section name="MsmqTransportConfig" type="NServiceBus.Config.MsmqTransportConfig, NServiceBus.Core" />
</configSections>

<MsmqTransportConfig
  InputQueue="subscriber.input"
  ErrorQueue="error"
/>

Versus

<appSettings>
  <add key="NServiceBus.MsmqTransport.InputQueue" value="subscriber.input"/>
  <add key="NServiceBus.MsmqTransport.ErrorQueue" value="error"/>
</appSettings>

Now this doesnt solve things like complex values with UnicastBusConfig

<configSections>
  <section name="UnicastBusConfig" type="NServiceBus.Config.UnicastBusConfig, NServiceBus.Core" />
</configSections>
<UnicastBusConfig>
  <MessageEndpointMappings>
    <add Messages="MyEventNamespace" Endpoint="publisher.input" />
  </MessageEndpointMappings>
</UnicastBusConfig>

But that could be solved by have one config section that handles any xml http://www.codinghorror.com/blog/2006/08/the-last-configuration-section-handler-revisited.html.

And yes this doesnt allow us to use the intellisense that comes with config sections. But to be honest I do not know of anyone who does this because it is too difficult http://stackoverflow.com/questions/378105/how-do-i-get-intellisense-in-app-config-for-a-custom-section And it doesnt work for us because we dont produce or deploy the xsd to enable it.

Thoughts?

johnsimons commented 11 years ago

I agree with @SimonCropp, I really fail to see the advantage of using custom config sections for simple types.

To me custom config sections are harder to code and we end-up with more code to maintain :disappointed:

:+1: to use appsettings instead

chrisbednarski commented 11 years ago

I haven't got an opinion one way or the other but the thing I usually forget about is configSource.

eg. NSB Studio does a lot of codegen into separate config files and links to those with configSource.

andreasohlund commented 11 years ago

How would we handle the IProvideConfiguration of T that we have today?

What would the equivalent be?

Sent from my iPhone

On 29 jul 2013, at 02:48, John Simons notifications@github.com wrote:

I agree with @SimonCropp, I really fail to see the advantage of using custom config sections for simple types.

To me custom config sections are harder to code and we end-up with more code to maintain

to use appsettings instead

— Reply to this email directly or view it on GitHub.

johnsimons commented 11 years ago
Configure.Endpoint.Audit(s => {
   s.ForwardReceivedMessagesTo("blah");
   s.TimeToBeReceivedOnForwardedMessages(TimeSpan.FromMinutes(3));
} );
andreasohlund commented 11 years ago

And I guess the IConfigSource override (the one where you override everything)could just accept a string key and return a T?

SimonCropp commented 11 years ago

My understanding was IProvideConfiguration was just a crutch for config we had not exposed explicitly with code?

johnsimons commented 11 years ago

I would obsolete IProvideConfiguration and IConfigurationSource, basically I would make our code only API more explicit like the example I gave.

andreasohlund commented 11 years ago

The is real use cases eg: override the encyption to read the key from a secure store instead of appconfig.

Another is to centralize configuration for all endpoints for certain sections

Sent from my iPhone

On 29 jul 2013, at 07:34, Simon Cropp notifications@github.com wrote:

My understanding was IProvideConfiguration was just a crutch for config we had not exposed explicitly with code?

— Reply to this email directly or view it on GitHub.

andreasohlund commented 11 years ago

We need to compile a list of use cases to make sure we cover them all good enough.

Sent from my iPhone

On 29 jul 2013, at 07:36, John Simons notifications@github.com wrote:

I would obsolete IProvideConfiguration and IConfigurationSource, basically I would make our code only API more explicit like the example I gave.

— Reply to this email directly or view it on GitHub.

udidahan commented 11 years ago

I’ve always wanted the intellisense for the config sections but couldn’t get them when we were releasing a simple zip. I think we can get it now.

From: John Simons [mailto:notifications@github.com] Sent: Monday, July 29, 2013 3:49 AM To: NServiceBus/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] Does Auditing need its own configuration section, rather than using UnicastBusConfig? (#1339)

I agree with @SimonCropp https://github.com/SimonCropp , I really fail to see the advantage of using custom config sections for simple types.

To me custom config sections are harder to code and we end-up with more code to maintain Image removed by sender. :disappointed:

Image removed by sender. :+1:to use appsettings instead

— Reply to this email directly or view it on GitHub https://github.com/NServiceBus/NServiceBus/issues/1339#issuecomment-21694710 .Image removed by sender.


No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3349 / Virus Database: 3209/6526 - Release Date: 07/27/13

udidahan commented 11 years ago

Good point.

From: Indu Alagarsamy [mailto:notifications@github.com] Sent: Sunday, July 28, 2013 11:17 PM To: NServiceBus/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] Does Auditing need its own configuration section, rather than using UnicastBusConfig? (#1339)

Currently, we have MasterNodeConfig which is separate. However, it applies for the Distributor feature. Should we have a config section called, MsmqScaleOutConfig? (since this is for Msmq only?) for all the distributor stuff?

— Reply to this email directly or view it on GitHub https://github.com/NServiceBus/NServiceBus/issues/1339#issuecomment-21690136 .Image removed by sender.


No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3349 / Virus Database: 3209/6526 - Release Date: 07/27/13