Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30625 closed Bug (fixed)

DatabaseCache backend raises TypeError if get/delete received key as integer.

Reported by: Hiroki Kiyohara Owned by: Hasan Ramezani
Component: Documentation Version: 2.2
Severity: Normal Keywords:
Cc: pope1ni Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Hiroki Kiyohara)

After Django 2.2, DatabaseCache backend will raise TypeError when get/delete method received integer key.

>>> cache = caches["..."]
<django.core.cache.backends.db.DatabaseCache object at 0x7f2d5ce37ac8>
>>> cache.get(1)
Traceback (most recent call last):
  File "/usr/lib/python3.7/code.py", line 90, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/home/hirokiky/.../venv/lib/python3.7/site-packages/django/core/cache/backends/db.py", line 52, in get
    return self.get_many([key], version).get(key, default)
  File "/home/hirokiky/.../venv/lib/python3.7/site-packages/django/core/cache/backends/db.py", line 60, in get_many
    self.validate_key(key)
  File "/home/hirokiky/.../venv/lib/python3.7/site-packages/django/core/cache/backends/base.py", line 245, in validate_key
    if len(key) > MEMCACHE_MAX_KEY_LENGTH:

I know it's not a bug. Because we should pass key as string.

key should be a str, and value can be any picklable Python object.

https://docs.djangoproject.com/en/2.2/topics/cache/#basic-usage

Before Django 2.2, key argument had been formatted by make_key method, and it would convert integer to string.
But now Django 2.2 will call validate_key before make_key, so it will raise TypeError if you pass integer key.

I think describing about this change on Django 2.2 release note is better
(as small backward incompatible change).

https://docs.djangoproject.com/en/2.2/releases/2.2/

Note: This behaviour will happen after this optimization.
https://code.djangoproject.com/ticket/29584

Change History (6)

comment:1 by Hiroki Kiyohara, 5 years ago

Description: modified (diff)

comment:2 by Carlton Gibson, 5 years ago

Cc: pope1ni added
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

OK, thanks for the report Hiroki.

I'm going to accept this as a documentation issue as you suggest.

But I'm not 100% sure that having validate_key() called before make_key() in abd0ad7681422d7c40a5ed12cc3c9ffca6b88422 is an intended behaviour.
(Elsewhere in django.core.cache.backends make_key() is called first.)

As such I'm just CC-ing Nick Pope, who was one of the reviewers on the change there for a second opinion. (Do we want to have a temporary variable to allow `make_key()` then `validate_key()` in this loop Nick? Thanks!)


comment:3 by Hasan Ramezani, 5 years ago

Has patch: set
Owner: changed from nobody to Hasan Ramezani
Status: newassigned

I made a PR to add note to backward incompatible changes section in Django 2.2 release note.

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In f69b3278:

Fixed #30625 -- Doc'd cache.get()/delete() behavior change in Django 2.2.

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 6e3ef984:

[3.0.x] Fixed #30625 -- Doc'd cache.get()/delete() behavior change in Django 2.2.

Backport of f69b32782e21642c6184162d888fcc17dd1dd85e from master

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 8a8ea93:

[2.2.x] Fixed #30625 -- Doc'd cache.get()/delete() behavior change in Django 2.2.

Backport of f69b32782e21642c6184162d888fcc17dd1dd85e from master

Note: See TracTickets for help on using tickets.
Back to Top