#14299 closed (wontfix)
Add additional cache.*_many functions
Reported by: | Michael Manfre | Owned by: | Michael Manfre |
---|---|---|---|
Component: | Core (Cache system) | Version: | dev |
Severity: | Keywords: | cache, add_many, offset_many | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
It would be helpful to have the cache framework allow backends to implement more bulk operations. Attached is a patch that adds support for add_many()
and
offset_many()
.
add_many()
To add multiple values more efficiently, use add_many()
to pass a dictionary
of key-value pairs. add_many()
returns a dictionary with all the keys passed
and their values being a bool indicating whether or not the value was added::
>>> cache.set('a', 1) >>> cache.add_many({'a': 2, 'b': 1}) {'a': False, 'b': 1}
offset_many()
To increment and decrement multiple values more efficiently, use
offset_many()
to pass a dictionary of key-value pairs, where the value is
the delta you would like apply to the existing value. offset_many()
returns a dictionary with all the keys passed and their new value. If a key is
not found or there was an error, its value will be False::
>>> cache.set_many({'a': 10, 'b': 10}) >>> cache.offset_many({'a': 1, 'b': -1, 'c': 1}) {'a': 11, 'b': 9, 'c': False}
Attached patch is against trunk, includes tests and docs.
Attachments (1)
Change History (7)
by , 14 years ago
Attachment: | django-cache-add_many.diff added |
---|
comment:1 by , 14 years ago
comment:2 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Django's cache API deliberately mimics memcached's API. Specifically, it doesn't implement any operations that aren't in memcached's API. Since these new _many operations aren't natively supported by memcached, they're not a good fit for Django's cache API.
If you're attached to 'em you can always write a custom cache backend and use that instead.
The incr/decr fixes should be filed a separate ticket; those are indeed bugs and should be fixed.
comment:3 by , 14 years ago
For clarification, I'm assuming you mean the Django API loosely mimics the python-memcached client library, which exposes delete_multi and set_multi to reduce connection latency, and not the memcached protocol commands? Is this design restriction documented anywhere? Are there any other unwritten or hidden decisions along these lines regarding the cache framework?
comment:4 by , 14 years ago
Patch needs improvement: | set |
---|
Feedback as I come across things -- I somehow didn't notice Jacob's already wontfix'd this ticket. Perhaps feedback is useful anyway. I leave Jacob to answer your previous questions.
- Did you test against other backends? I see you've implemented in base, but it's not clear to me that base will work in all backends.
- Dummy .set_many should return the empty dict rather than None
- Why the change to smart_str in memcache incr/decr? Unrelated issue?
- Replace: "
add_many()
returns a dictionary with all the keys passed and their values being a bool indicating whether or not the value was added" with "
add_many() returns a dictionary containing all keys passed in and boolean values indicating whether the value was added."
- test_add_many and test_offset_many are repeated in your diff. Please remove the dupes.
- move comment "# add_many takes a second
timeout
parameter" below the call to delete_many.
- In test_add_many, please ensure to test both cases where a key does exist and where it does not.
- In offset_many, I'm not sure about returning booleans for a failed incr/decr. Is this something you've seen in other backends?
comment:5 by , 14 years ago
jdunck, thanks for the feedback. I'm not sure it's worth the time to update the patch given the wontfix.
- Tests in the base mixin run for all of the backends, except dummy.
- Agreed.
- Unrelated issue see #14315
- ok
- One set is for the dummy backend and the second set is for the rest of them.
- ok
- ok
- A more conforming approach would be to return a dict containing the new values. Any keys with errors would be omitted or set to None.
Patch contains a fix for memcached's incr() and decr(), which raise the 'Key not found' ValueError if delta is negative. If needed, I can split that fix off to a separate ticket and change offset_many() to explicitly call either incr() or decr().