Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A discussion of the current config API in v5 #2240

Closed
SimonCropp opened this issue Jul 22, 2014 · 4 comments
Closed

A discussion of the current config API in v5 #2240

SimonCropp opened this issue Jul 22, 2014 · 4 comments

Comments

@SimonCropp
Copy link
Contributor

So in v4 a config story might look like this

class UnobtrusiveMessageConventions : IWantToRunBeforeConfiguration
{
    public void Init()
    {
        Configure.Instance
                 .DefiningMessagesAs(t => t.Namespace != null && t.Namespace.StartsWith("VideoStore") && t.Namespace.EndsWith("RequestResponse"));
    }
}

class EndpointConfig : IConfigureThisEndpoint, IWantCustomInitialization
{
    public void Init()
    {
        Configure.With()
            .UseNHibernateTimeoutPersister();
    }
}

or this

class EndpointConfig : IConfigureThisEndpoint, IWantCustomInitialization
{
    public void Init()
    {
        Configure.With()
            .DefiningMessagesAs(t => t.Namespace != null && t.Namespace.StartsWith("VideoStore") && t.Namespace.EndsWith("RequestResponse"));
            .UseNHibernateTimeoutPersister();
    }
}

the new API in the v5 beta would be

class EndpointConfig : IConfigureThisEndpoint, INeedInitialization
{
    public void Customize(ConfigurationBuilder builder)
    {
        builder.Conventions(c => c
            .DefiningMessagesAs(t => t.Namespace != null && t.Namespace.StartsWith("VideoStore") && t.Namespace.EndsWith("RequestResponse")));
    }

    public void Init(Configure config)
    {
        config.UsePersistence<NHibernate>();
    }
}

The differences

So what do people think of the following?

No more "unobtrusive configuration"

So in v4 it was possible to do "true unobtrusive configuration". By that I mean you could implement IWantToRunBeforeConfiguration, do any configuration, then have that picked up by NSB assembly/type scanning and it would be applied. In v5 this is no longer possible. The closest that can be achieve is a helper method that is explicitly called from within the IConfigureThisEndpoint.Customize.

A split forced split of the configuration code

With the new v5 approach we are forcing people to split their configuration code between two methods. There is problematic since there is no way someone can guess at what types of configuration should be in each method. The fact that this split is needed is an implementation detail that should not be inflicted on the user.

The extension points are split over two interfaces

The configuration story is split across two interfaces IConfigureThisEndpoint and INeedInitialization. Which is strange when most people will need to implement both. Also the naming of INeedInitialization in no way implies it should be used to manipulate the configuration on the endpoint.

The extension points are split over two assemblies

The configuration story is split across two assemblies NServiceBus.Core.dll and NServiceBus.Host.exe which is very confusing.

There can only be one IConfigureThisEndpoint but there can be multiple INeedInitialization

Is this by design? If these interfaces will most likely always be paired why the difference?

IConfigureThisEndpoint must exist on the EndpointConfig class but INeedInitialization can exist anywhere

Again. Is this by design? If these interfaces will most likely always be paired why the difference?

@indualagarsamy
Copy link
Contributor

@andreasohlund - thoughts?

@andreasohlund
Copy link
Member

I'd say we need to fix this. What are our options?

  • Making the IConfigureThisEndpoint a abstract base so we can provide more
    smarts
  • Revert back to returning the Configure so but that gets us back to the
    issue where all defaults set by the host gets lost
  • Obsolete the host :)

On Tue, Jul 22, 2014 at 6:16 PM, Indu Alagarsamy notifications@github.com
wrote:

@andreasohlund https://github.com/andreasohlund - thoughts?


Reply to this email directly or view it on GitHub
#2240 (comment)
.

@indualagarsamy
Copy link
Contributor

I say we port the rest of the stuff that we haven't to the Customize method, so all of the configuration can be applied in the IWantToConfigureThisEndpoint's Customize method. So the v5 story will be a super clean configuration story that gets rid of the ugly ordering.
Although obsoleting the host is very tempting :)

@andreasohlund
Copy link
Member

Partly fixed in v5, please reopen fine grained issues if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants