Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Caching APIs for adding services need to take in config lambda #152

Closed
Eilon opened this issue Feb 12, 2016 · 8 comments
Closed

Caching APIs for adding services need to take in config lambda #152

Eilon opened this issue Feb 12, 2016 · 8 comments
Assignees
Milestone

Comments

@Eilon
Copy link
Member

Eilon commented Feb 12, 2016

The various services.AddXYZCaching() methods are all parameterless - they need to take in a configuration lambda. Right now you have to call services.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.

@Eilon
Copy link
Member Author

Eilon commented Feb 12, 2016

@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?

@divega
Copy link

divega commented Feb 12, 2016

Adding @rynowak who last time did a great job getting all these ducks in a line 😄

@rynowak
Copy link
Member

rynowak commented Feb 16, 2016

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.

@muratg
Copy link

muratg commented Mar 2, 2016

@rynowak Did you get a chance to look into this?

@Eilon
Copy link
Member Author

Eilon commented Mar 3, 2016

@muratg we have a meeting on this tomorrow, I'll add you to it.

@javiercn
Copy link
Member

javiercn commented Mar 3, 2016

@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

@javiercn
Copy link
Member

javiercn commented Mar 3, 2016

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.

@javiercn
Copy link
Member

javiercn commented Mar 8, 2016

a6c044c

@javiercn javiercn closed this as completed Mar 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants