django-ninja icon indicating copy to clipboard operation
django-ninja copied to clipboard

Support caching.

Open mom1 opened this issue 2 years ago • 13 comments

Hello.

This is an implementation of caching by suggestion #345.

I did it only for get, but I think it will not be difficult to expand.

I hope it will be useful :)

mom1 avatar Jan 31 '22 17:01 mom1

Why not just implement this as a decorator rather than an operator argument?

SmileyChris avatar Jan 31 '22 20:01 SmileyChris

A lot of response options from the view. You need to understand what is expected from the view for serialization and caching. An obvious example is when a queryset is returned from a view, it cannot be sent to the cache directly.

mom1 avatar Jan 31 '22 20:01 mom1

Reading your reasons on the other ticket. I'm not sure I agree that it means you couldn't have it as a decorator still. Just make the decorator return a wrapping cache class, which then the operator function can check for and still use to determine the cache status.

I guess I'm just not a fan of adding more and more arguments to the operators...

SmileyChris avatar Jan 31 '22 20:01 SmileyChris

Reading your reasons on the other ticket. I'm not sure I agree that it means you couldn't have it as a decorator still. Just make the decorator return a wrapping cache class, which then the operator function can check for and still use to determine the cache status.

I guess I'm just not a fan of adding more and more arguments to the operators...

I don't quite understand how it can be done differently. Can you provide some kind of outline or meta code?

mom1 avatar Jan 31 '22 20:01 mom1

@mom1 @SmileyChris

Indeed... :( there a lot of stuff still due which feels like good place to put to get/post/etc methods... But I think it will not lead to a good design and there always be a case when something is missing...

On the other hand I totally understand the issue with decorators that already exist in Django ecosystem that is not compatible with django-ninja (simply because in 99% of the cases django-ninja do not return HttpRequest objects)

I guess some workaround can be a special decorator that excuses AFTER function result serialised to Django response:


@api.get('/foo')
@decorate(cache_func, logging_func, any_otther_decorator)
def foo(request, a: int, b: str):
     return Model.objects.all()

the idea of decorate is it will add some attribute to operation that when executed it will wrap around call before foo execution and after response is serialised...

vitalik avatar Feb 01 '22 13:02 vitalik

@vitalik I fully agree with this. I tried to implement this, but I ran into a problem: for serialization, you need to know the response, which is in the Operation class, and the serialization logic too. When processing the request, the router does not participate and it is impossible to get the desired operation.

To avoid code duplication

def cache_page(f):
    @wraps(f)
    def wrapper(request, *args, **kwargs):
        key  = `your key from request`
        result = django_cache.get(key)
        if result:
             return result
        result = f(request, *args, **kwargs)
        # here you need to magically serialize result using MyModel
        django_cache.set(key, result, timeout=30)
        return result

    return wrapper

@api.get('/foo', response={200: MyModel})
@cache_page(response={200: MyModel})
def foo(request, a: int, b: str):
     return Model.objects.all()

I came to the suggested solution

the idea of decorate will require more changes and I don't fully understand it.

mom1 avatar Feb 01 '22 14:02 mom1

Hmm. I understand. _ninja_contribute_to_operation

mom1 avatar Feb 04 '22 13:02 mom1

@mom1

yeah, I guess you can use _ninja_contribute_to_operation callback to change decorate Operation.run

just keep in mind that run can be both sync and async and it should be checked before decorating

vitalik avatar Feb 04 '22 14:02 vitalik

@vitalik You can look at my commit, would such a check be enough?

mom1 avatar Feb 07 '22 08:02 mom1

@mom1 I think it will not work - basically to support async views you need to implement two wrappers (sync and async)

you need to add there tests as well (where you've put # pragma: no cover)

see how async tests are done here - https://github.com/vitalik/django-ninja/blob/master/tests/test_async.py

vitalik avatar Feb 07 '22 08:02 vitalik

@vitalik Can you take another look?

mom1 avatar Feb 07 '22 14:02 mom1

Hi is this stale ?

Can we get some progress on this? I am in a situation where i need caching support badly

baseplate-admin avatar Apr 02 '23 04:04 baseplate-admin

It looks like this is stale. There are alternatives to using caching as mentioned in the docs here and in Vitalik's comment here

jaredleishman avatar Feb 12 '24 05:02 jaredleishman