search

adamchainz / django-perf-rec

Keep detailed records of the performance of your Django code.
MIT License
347 stars 25 forks source link

Current django_perf_rec implementation is not compatible with the usage of named "key" parameter to set_cache function #536

Closed trasfract closed 1 year ago

trasfract commented 1 year ago

Python Version

3.9.16

Django Version

3.2.16

Package Version

4.21.0

Description

The django.core.cache definition of set_cache allows the naming of all parameters. cache.set(key="key", value="value") is therefore acceptable

See https://docs.djangoproject.com/en/3.2/topics/cache/#django.core.caches.cache.set

However, django_perf_rec's current implementation is incompatible with this approach as it directly accesses args[0] and considers it as the "key" parameter.

In cache.py, the line which I believe is faulty is the following: key_or_keys = args[0] This should be replaced with a more robust approach: key_or_keys = args[0] if args else kwargs['key'] or, better yet(?) declare key as a named parameter directly in the parameters of the function overload.

adamchainz commented 1 year ago

Happy to accept a PR here, even if you can only get part way adding a test.

trasfract commented 1 year ago

I have opened a PR for this issue. Don't hesitate and review it :-) Happy to get some feedback

trasfract commented 1 year ago

@adamchainz Hello! Did you get a chance to have a look at the PR I have opened to solve this issue? https://github.com/adamchainz/django-perf-rec/pull/545

adamchainz commented 1 year ago

Please don't make such "nudge" comments, they aren't helpful. I do open source when I can.