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

cachimo.clear() should also clear all timeouts #1

Closed
gr2m opened this issue Feb 10, 2018 · 7 comments
Closed

cachimo.clear() should also clear all timeouts #1

gr2m opened this issue Feb 10, 2018 · 7 comments
Labels
enhancement ✨ New feature or request

Comments

@gr2m
Copy link

gr2m commented Feb 10, 2018

Hey Benas, thanks for the neat little key/value memory store!

I have one feature request: when testing with cachimo, the process will not end until all timeouts are done. It would be nice if cachimo.clear() would clear all timeouts, too. I could try to add that if this is within the scope of the library?

@svipas
Copy link
Owner

svipas commented Feb 12, 2018

Hey @gr2m, sorry for late response.

Very good suggestion! I think we can store them inside Set or simple Array. I think it makes sense to have function cachimo.terminate() or cachimo.stop() which clears all timeouts, but all key/values will be inside cache. What do you think?

P.S. If you want you can open PR or I can do it. 👷

@gr2m
Copy link
Author

gr2m commented Feb 12, 2018

Happy to send a PR :)

I think making cachimo.clear() also stop all timeouts would be good enough, at least for my use case. Or do you see a reason where you want to clear the cache but keep the timeouts intact?

@svipas
Copy link
Owner

svipas commented Feb 13, 2018

@gr2m What I mean is cachimo.clear() will clear everything (keys/values and timeouts). But also we need to have cachimo.terminate() or cachimo.stop() which will clear only timeouts. It's good if developer want to keep keys/values, but want to stop/terminate all timeouts.

@gr2m
Copy link
Author

gr2m commented Feb 13, 2018

If I understand you correctly, it would work like this?

cachimo.put('key', 'value', 1000)
cachimo.stop()
setTimeout(() => {
  cachimo.get('key') // 1000
}, 2000)

calling cachimo.stop() would cancel all timeouts for data currently stored in cache? I still don't see a use case for that, I must miss something?

Okay if I go ahead and send a pull request to update cachimo.clear() only for now?

@svipas
Copy link
Owner

svipas commented Feb 15, 2018

@gr2m Yeah for now update cachimo.clear(), but what I mean is cachimo.stop() will clear all timeouts not the data. Imagine you use cachimo.put('key', 'value', 60e3); which will remove your key with value after 60 seconds and you changed your mind and you want to keep that key with value so you use cachimo.stop() or cachimo.terminate() to clear all timeouts and your key with value will be available because it only clears timeouts not the data. So you can easily do again cachimo.get('key'); to get value. This is only usable if timeouts are more than 1 minute. I even see it makes sense to have cachimo.stop('key') to stop not all timeouts but only for given key. But for now you can make only cachimo.clear() to clear all timeouts too.

But keep this issue open because of our discussion.

Thanks again @gr2m ! 👍

@svipas
Copy link
Owner

svipas commented Feb 17, 2018

I already created and merged these PRs:

/cc @gr2m

@svipas svipas closed this as completed Feb 17, 2018
@gr2m
Copy link
Author

gr2m commented Feb 17, 2018

thanks a lot!

@svipas svipas added the enhancement ✨ New feature or request label Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants