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

Introduce Flipper::Adapters::FallbackToCached #860

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonmagic
Copy link

@jonmagic jonmagic commented Apr 8, 2024

Why?

For my use case I need to be able to cache values each time features, get, get_multi, or get_all are called but only use those cached values if subsequent calls to them fail. It should raise an error if no cached values are there to use.

How?

I introduced a new adapter called Flipper::Adapters::FallbackToCached that inherits from the Memoizable adapter and wraps another adapter. It updates the implementation of features, get, get_multi, and get_all to call the primary adapter and caches the values. If the primary adapter raises an error, it will return the cached value instead. If there is no cached value, it will raise the error.

Please note you have to disable memoize when configuring Flipper so that it doesn't use the Memoizable adapter like it normally would.

@jonmagic jonmagic force-pushed the use-latest-from-memory-if-primary-source-fails branch 2 times, most recently from 7f1a555 to 862af8e Compare April 8, 2024 21:08
@jonmagic jonmagic force-pushed the use-latest-from-memory-if-primary-source-fails branch from 862af8e to 0bc8cb9 Compare April 8, 2024 21:20
@jnunemaker
Copy link
Collaborator

I think my only issue with this is the inheriting from memorizable and turning memoize off.

It might be just because I haven't started from scratch on it to see how much is shared.

I think I'll take a stab at it from scratch to see the difference.

@jonmagic
Copy link
Author

turning memoize off.

I think you have to turn this off or it will add the memoizable adapter as the first in the stack and it defeats the purpose of this change.

@jnunemaker
Copy link
Collaborator

Memoizable usually just sits in front and falls back to the original adapter. So if your original falls back to in memory cache it should just work. Maybe something else weird was at play. I'll have to kick the tires.

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

Successfully merging this pull request may close these issues.

2 participants