-
Notifications
You must be signed in to change notification settings - Fork 216
Caching APIs for adding services need to take in config lambda #152
Comments
@divega @HaoK @lodejard @davidfowl @DamianEdwards I wonder if we have more inconsistencies around? I presume we need to have someone take a look at all our services extension methods? |
Adding @rynowak who last time did a great job getting all these ducks in a line 😄 |
I'm here! Yeah, we should take a look. I seem to recall identifying 3-4 things that needed to change before... I'll see if I can dig up that mail. Wouldn't hurt to do a fresh pass too. I'll follow up on that and open issues. |
@rynowak Did you get a chance to look into this? |
@muratg we have a meeting on this tomorrow, I'll add you to it. |
@Eilon I'm already covering this as part of https://github.com/aspnet/Caching/pull/156/files. Se might need to add options to inmemory |
Actually, inmemory needs the options flavor. I've been looking at this specifically on all the repositories where I've normalized the service collection extensions. For inmemory specifically, I'm not sure what the value for the IClock implementation would be. I believe that fits better as a service. |
The various
services.AddXYZCaching()
methods are all parameterless - they need to take in a configuration lambda. Right now you have to callservices.Configure<XYZCacheOptions>(options => { options.FooSetting = "value"; });
to configure them.E.g.:
Note: The SQL Server services extension method does the right thing: https://github.com/aspnet/Caching/blob/dev/src/Microsoft.Extensions.Caching.SqlServer/SqlServerCacheExtensions.cs#L23-L25
@divega this is what I mentioned to you yesterday.
The text was updated successfully, but these errors were encountered: