search

aspnet / MusicStore

[Archived] MusicStore test application that uses ASP.NET/EF Core. Project moved to https://github.com/aspnet/AspNetCore
1.3k stars 878 forks source link

Add cache Timeout configuration #609

Closed martinpf closed 8 years ago

martinpf commented 8 years ago

This PR addresses the latest feedback from:

603

608

Fixes coding convention issues Readds the console logging (logging will be addressed in another PR) changes cacheTimeout config item name to be more descriptive Reinstates original cacheTimeout value

dnfclas commented 8 years ago

Hi @martinpf, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! You've already signed the contribution license agreement. Thanks! We will now validate the agreement and then real humans will evaluate your PR.

TTYL, DNFBOT;

sajayantony commented 8 years ago

@muratg Do you want to merge this into MusicStore dev? @martinpf please squash all the commits and update the branch.

muratg commented 8 years ago

@kichalla Could you review and merge?

Eilon commented 8 years ago

:watch:

sajayantony commented 8 years ago

ping @kichalla

kichalla commented 8 years ago

An alternative approach I am thinking about is to create a wrapper type for IMemoryCache where this type gets IMemoryCache and appsettings injected to it and does similar logic as the current extensions do. So where ever you want to use cache just get the wrapper type injected and that abstracts the details of checking the app setting etc.

sajayantony commented 8 years ago

@kichalla can we do that on top of this change or do you already have it.

Eilon commented 8 years ago

@SajayAntony the change @kichalla would be proposes would be what we think is a cleaner way of doing this change.

sajayantony commented 8 years ago

@Eilon closing this PR since the #620 provides the ability to disable the cache which is much simpler.