-
-
Notifications
You must be signed in to change notification settings - Fork 124
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
feat: add result cache consumer #341
base: master
Are you sure you want to change the base?
Conversation
@rcarriga any feedback on this? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Overall love the idea, just some minor fixes/changes. I also think this would make sense as part of the state consumer and in the state consumer config we can have something like persist_results: boolean
, rather than a whole separate consumer for it. It can still be a separate module to keep it tidy
I have added a config to auto load cache on startup; not sure now what functionality you would expect from a persist_results boolean. It should automatically cache all results as soon as they come in; rather than having to execute a command to cache it? |
Attempt to solve #195.
Need some feedback: the client interface does not seem to expose any way to update results. Any pointers on how to solve this? I have some ideas but not super happy with them: