Adds LimitLengthPlugin, tests missing. #721
Conversation
| a = list((client._handlers[k].when(), k) | ||
| for k in client._handlers.keys()) | ||
| a.sort() | ||
| for _, k in a[:current_length - self.max_length]: | ||
| await client.delete(k) |
There was a problem hiding this comment.
This looks like a pretty inefficient way to handle it, so not sure we'd want to include this in the library.
But, the bigger problem, if I'm not mistaken, is that this only works if using ttl. It's probably more likely that a user is going to want to limit the size when they don't use ttl (as using ttl already cleans up the cache and helps limits the size).
There was a problem hiding this comment.
No problem. I'll try a different approach. Please close this pull request.
There was a problem hiding this comment.
As mentioned in the issue, maybe it's worth creating a custom backend with Theine?
You could then publish that backend somewhere and add it to the README here.
The memory backend here is rather simple and probably best for testing things without needing to have a redis/memcache server available, rather than being relied upon as a production backend.
There was a problem hiding this comment.
Yes, handling record expiration efficiently is non-trivial, using a dedicated tool seems sensible. I have still not made sense of most of the codebase - the unit test part, for example - so I'm not at a position to write a backend for that. We need just the basic functionality for an API we are deploying at my employer, and he is fine with me contributing it back to the project, hence the merge request. I agree that it does not pass the bar for a production library like aiocache. I am in crunch mode for more than a week now, stamina running really low.
What do these changes do?
This addresses #498 by adding a plugin that limits the amount of cached records to max_length for memory based caches.
"max_size" is not used since it can be more easily mistaken by a limit in bytes instead of number of records.
After a record is added, if the cache is larger than the set capacity, the records closest to expiration are deleted until the cache returns to the size limit.
Are there changes in behavior for the user?
Only if the user decides to use it, in which case the user will create the caches using
plugins=[LimitLengthPlugin(max_length=whatever),...]. It changes nothing otherwise.Related issue number
#498
I have added some code to test basic functionality, but I have no idea how to properly implement the tests. I will be happy to implement the tests if you can point me in the right direction.
Checklist
CHANGESfolder<issue_id>.<type>(e.g.588.bugfix)issue_idchange it to the pr id after creating the PR.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.Fix issue with non-ascii contents in doctest text files.