Skip to content
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

Add matching option on returned result to Nebulex.Caching #55

Closed
polmiro opened this issue Oct 18, 2019 · 2 comments
Closed

Add matching option on returned result to Nebulex.Caching #55

polmiro opened this issue Oct 18, 2019 · 2 comments

Comments

@polmiro
Copy link
Contributor

polmiro commented Oct 18, 2019

There is a use that I believe is fairly common that I do not think fits in the Nebulex.Caching macro DSL.

Sometimes one wants to conditionally cache a value based on the result of the underlying function logic. This is easy to achieve by using the lower-level APIs Cache.get and Cache.set but it is fairly verbose and repeatable. Also, I was really enjoying the Nebulex.Caching DSL.

In my specific use case, I have a cache-through layer in front of a REST API and I only want to cache the results when the REST query underneath succeeds. Sometimes there are transient error responses or short periods of time where this API is down.

I added an optional function argument called match (for lack of better naming at the moment) to defcacheable that receives the result of the logic and returns a boolean value. Based on that, the value will be cached or not cached. For example:

defcacheable get_user(id), cache: Cache, key: {User, id}, match: &match?/1 do
  Tesla.get("users/#{id}")
end

defcacheable get_users(id), cache: Cache, key: {User, :list}, match: &match?/1 do
  Tesla.get("users")
end

# Successful requests will be cached
def match?({:ok, _}), do: true
# Failed requests will not be cached
def match?(_), do: true

If you think this would be worth adding I would be happy to write a PR, I already have something I can base it off. It is fairly straight-forward.

By the way, thanks for such a great library, cheers 🙇 !

@cabol
Copy link
Owner

cabol commented Oct 18, 2019

Hey @polmiro, that looks like a very nice feature!

If you think this would be worth adding I would be happy to write a PR, I already have something I can base it off. It is fairly straight-forward.

Absolutely it's worth it, please send the PR! Again, this would be a very nice and useful feature, thanks!

By the way, thanks for such a great library

Thank you so much, glad to hear that!

@polmiro
Copy link
Contributor Author

polmiro commented Oct 21, 2019

Closing in favor of the PR then :)

@polmiro polmiro closed this as completed Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants