-
Notifications
You must be signed in to change notification settings - Fork 68
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
#145 Define ICache, centralize implementation in Equinox.Core #161
Conversation
Thanks for doing this work; from first glance it looks pretty complete. Quick thoughts:
Probably won't get to take a deeper look today but hopefully shortly |
Some follow-on points:
despite suggesting introducing new packages, I'm keen to
This is making me think the best approach should be along the following lines
For now, I'd be comfortable with working step 1, but prior to taking most of the other steps, I'd be looking to:
Does any of this seem reasonable to you? |
I'm looking to implement #157 and #61 soon - ideally that'd be after this gets rebased and/or cherrypicked onto master now #164 is complete. |
ad6c052
to
b6a56e1
Compare
Agree with the points above. I've updated the PR and rebased it on the latest master. The interface (and related base types) were moved to Core package (to a separate Cache.fs file), and implementations are made part of EventStore.fs and Cosmos.fs. My only issue with that is that those packages remain dependent on System.Runtime.Caching - even if another caching library is used, but it's something I can live with 😄. |
Thanks! Given it wouldn't materially affect ones dependency tree, what would you think about moving to a single impl of Cache as |
All up for it. I'll update the pull request. |
Done, please take a look. |
Looks great. Some minor picky comments regarding my/house style adherence added. |
7aa4e1b
to
c19f646
Compare
c19f646
to
4f96845
Compare
🎉Looks perfect now; thanks for bearing with me over the course of all the iterations. |
I've finally had some free time to hack something together. Here is the summary of changes:
ICache
interface (note that it'sasync
based). Moved theCacheEntry
type nearby, changed the Token.supersedes logic to be customizable. Added newCacheEntryOptions
which is a more abstract way to configure the behavior of cache items (relative/absolute expiration).System.Runtime.Caching
integration into a separate project. The implementation remains the same.Microsoft.Extensions.Caching
implementation. Couldn't find a good way to customize the way it calculates object size (which wasn't terribly slow) so added a way to customize it via constructor parameter.The distributed cache implementation is also pretty straightforward given that there is an abstraction here now.
Looking forward to feedback!