[Django] #36020: redis cache backend serializing floats

10 views
Skip to first unread message

Django

unread,
Dec 17, 2024, 5:52:48 AM12/17/24
to django-...@googlegroups.com
#36020: redis cache backend serializing floats
--------------------------+------------------------------------------------
Reporter: amirreza | Type: Cleanup/optimization
Status: new | Component: Core (Cache system)
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------+------------------------------------------------
hi
so as you know django's redis backend
[https://github.com/django/django/blob/main/django/core/cache/backends/redis.py#L19-L27]
doesn't serialize int values, this is to support atomic integer operation
as discussed at
[https://github.com/django/django/pull/14437#issuecomment-915046037] and
below.

but redis also has float operations, such as `incrbyfloat`, and since
django serializes floats these operations are not possible, at least not
without some hassle

we can easily address this by not serializing floats

worth mentioning that i've recently done the same thing in `django-valkey`
[https://github.com/amirreza8002/django-
valkey/blob/main/django_valkey/base_client.py#L527-L552], and from what i
can tell, it makes no difference for users and doesn't break anything.

let me know what you think
--
Ticket URL: <https://code.djangoproject.com/ticket/36020>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 17, 2024, 9:19:38 AM12/17/24
to django-...@googlegroups.com
#36020: redis cache backend serializing floats
-------------------------------------+-------------------------------------
Reporter: amirreza | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Core (Cache system) | Version: 5.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* cc: Nick Pope (added)
* resolution: => wontfix
* status: new => closed

Comment:

In investigating this ticket, I found something on `django-redis` on
`incrbyfloat`: https://github.com/jazzband/django-
redis/issues/311#issuecomment-412816573

> [This] is not a redis client, [this] is a django cache backend. It
should implement the interface of django cache backend. If you want access
to redis funcionality you can access to redis connection directly using
http://niwinz.github.io/django-redis/latest/#_raw_client_access that
already exposes all redis functionality and we dont need reimplement it.

This sounds like a request to make it easier to interact with other redis
operations. I don't think that's the goal of the cache backend
--
Ticket URL: <https://code.djangoproject.com/ticket/36020#comment:1>

Django

unread,
Dec 17, 2024, 9:35:22 AM12/17/24
to django-...@googlegroups.com
#36020: redis cache backend serializing floats
-------------------------------------+-------------------------------------
Reporter: amirreza | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Core (Cache system) | Version: 5.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by amirreza):

hi
fair points
but to clarify, this is not a ticket to implement `incrbyfloat` or similar
methods, it's for not serializing floats, which will make float operations
easier, and i feel like it's more consistent since integers are not
serialized

p.s: i don't use redis backend at the moment, so i don't really insist on
this, if django team wants, i'll make a PR for this, if not, we move on
with life :)
--
Ticket URL: <https://code.djangoproject.com/ticket/36020#comment:2>

Django

unread,
Dec 17, 2024, 9:56:21 AM12/17/24
to django-...@googlegroups.com
#36020: redis cache backend serializing floats
-------------------------------------+-------------------------------------
Reporter: amirreza | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Core (Cache system) | Version: 5.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce):

I understand that 🙂 this is to add a float special-case on top of the
existing int special-case.

Note that folks were confused why serialization is skipped in the `int`
special-case for redis (see discussion in #33361).
Adding a float special-case (when I don't believe we support any float
operations) feels even harder to justify.

I'm not an expert here and some folks might chime in. This can also be
discussed further on the [https://forum.djangoproject.com/c/internals/5
Django Forum]
--
Ticket URL: <https://code.djangoproject.com/ticket/36020#comment:3>
Reply all
Reply to author
Forward
0 new messages