Feature/add cache decorator #313
Conversation
AMontagu
commented
Jul 26, 2024
•
AMontagu
commented
- Test on real app
- Clean code
- Write forgotten test about change default settings value for cache
- Test automatique wrapper around django decorator
- Implement an auto clean cache decorator from django signals
- Write documentation*
- Make changelog. Put inside the inheritance of django request/response. The headers proxy, the http_to_grpc_decorator, cache feature and the fact that metadata in test should match real grpc convention, drop support of django version and python versions
AMontagu
force-pushed
the
feature/addCacheDecorator
branch
6 times, most recently
from
bc50bac to
5ae15cb
Compare
AMontagu
force-pushed
the
feature/addCacheDecorator
branch
from
eb8d4d6 to
fd1aeb3
Compare
django_socio_grpc/decorators.py
Outdated
| :param senders: The senders to listen to the signal | ||
| :param invalidator_signals: The django signals to listen to delete the cache | ||
| """ | ||
| if not key_prefix: |
There was a problem hiding this comment.
key_prefix has no default value so this case is only when the user puts ""
There was a problem hiding this comment.
yes it is to avoid user puts "" by mistake
| return grpc_request_metadata | ||
|
|
||
| def get_query_params(self, grpc_request: Message): | ||
| def get_query_params(self, grpc_request: Message) -> dict: |
There was a problem hiding this comment.
| def get_query_params(self, grpc_request: Message) -> dict: | |
| def get_query_params(self, grpc_request: Message) -> dict[str, str]: |
django_socio_grpc/decorators.py
Outdated
| :param invalidator_signals: The django signals to listen to delete the cache | ||
| """ | ||
| if not key_prefix: | ||
| logger.warning( |
There was a problem hiding this comment.
why without a prefix does it not delete only this route from cache ?
There was a problem hiding this comment.
Because if not using Redis there is no way to clean multiple key from cache without knowing them exactly.
So we clear all the cach: https://github.com/socotecio/django-socio-grpc/pull/313/files#diff-e320866935eaa353b47eb8ff96e0bbcb20aa7fa3b0ae29b08681b63604816cb6R308
| @@ -362,6 +405,10 @@ async def wrapped(*args, **kwargs): | |||
| ) | |||
| return self | |||
|
|
|||
| async def with_call(self, request=None, metadata=None): | |||
There was a problem hiding this comment.
it doesn't seem to exist in async
There was a problem hiding this comment.
good catch. I changed it
… work and then get inspiration by code from another repo
…nging pagination or filters
… error in real app
AMontagu
force-pushed
the
feature/addCacheDecorator
branch
from
117e15f to
dbb6458
Compare
| @@ -123,6 +124,11 @@ def __set_name__(self, owner, name): | |||
| if "_decorated_grpc_action_registry" not in owner.__dict__: | |||
| owner._decorated_grpc_action_registry = {} | |||
| owner._decorated_grpc_action_registry.update({name: self.get_action_params()}) | |||
| # INFO - AM - 22/08/2024 - Send a signal to notify that a grpc action has been created. Used for now in cache deleter | |||
| grpc_action_set.send(sender=self, owner=owner, name=name) | |||
There was a problem hiding this comment.
With this you access only the parent class which defines the action, this should be in the register method
django_socio_grpc/decorators.py
Outdated
| # INFO - AM - 22/08/2024 - http_to_grpc is a decorator but as it is returned from a decorator (to be able to access func) we need to call it directly to have the actual decorator | ||
| return http_to_grpc( | ||
| method_decorator( | ||
| cache_page(timeout, key_prefix=key_prefix, cache=cache), name="cache_page" |
There was a problem hiding this comment.
I don't think the nonlocal will make this work, the functions are already resolved and returned. This could be done by modifying action.function = http_to_grpc(...(action.function)) when the signal is triggered
…8n and use logger instead of settings
