Minion icon indicating copy to clipboard operation
Minion copied to clipboard

Remove the need for static MinionConfiguration

Open matt-psaltis opened this issue 5 years ago • 2 comments

We often deploy logically separate services as a single deployment unit using a single service or web app as the host and then boot up the logically separate services within the same application / app domain.

Due to the static Configuration property of the MinionConfiguration, this can cause issues when the MinionConfiguration is configured by multiple logical services. I've had a cursory look and it appears that the static nature of the MinionConfiguration can be fairly easily removed. This would mean that a MinionConfiguration instance is constructed and passed into BatchEngine for example as a constructor parameter.

var minionConfiguration = new MinionConfiguration();
minionConfiguration.UseSqlStorage("<connection string>");

var engine = new BatchEngine(minionConfiguration);
engine.Start();

Is this something you'd be open to adding?

matt-psaltis avatar Mar 14 '19 01:03 matt-psaltis

Hi! As of right now, you can use the BatchEngine without the static configuration, just use the other constructor to inject all needed dependencies.

I'm thinking of using the generic host (https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/generic-host?view=aspnetcore-2.2) as the primary hosting model for Minion. Which I hope will allow for a cleaner way to configure the BatchEngine (Thru DI), but I haven't looked into this yet.

I'm open to suggestions on how to make this a better experience :)

pug-pelle-p avatar Mar 14 '19 07:03 pug-pelle-p

No worries, my concern with using the second constructor is it bypasses the argument validation rules particularly in batch engine. Additionally the configuration extension methods don't pass the MinionConfiguration instance represented by "this" in the extension method to the underlying object. This has the effect of the wrong date service being applied in the case of say the SQL Storage object. It just reduces the syntax sugar usefulness a bit.

We're having a lot of success with generic host builder. It might be worth looking at what's coming in net core 3.0 with the consolidation of generic host and Web host builder.

Thanks for feedback

matt-psaltis avatar Mar 14 '19 09:03 matt-psaltis