Cache service usage endpoint #4666
Conversation
kpi/views/v2/service_usage.py
Outdated
| @method_decorator(cache_page(60 * 30), name='retrieve') | ||
| @method_decorator(cache_page(60 * 30), name='list') |
There was a problem hiding this comment.
Let's use a setting for this (which could even be configurable with env variable, then fallback on your default). We could have a shorter timeout in dev settings than prod.
Maybe 30 minutes is a little bit too long, what about reverting it to 15?
There was a problem hiding this comment.
I've extracted this out to ENDPOINT_CACHE_DURATION. This setting is used by /service_usage/ and /stripe/products/, but that can be refactored if we later need to set those cache settings independently.
The only thing I'm not sure about is using a shorter timeout in dev settings; I'm worried that could make it easier to miss unwanted results of caching while debugging.
kpi/views/v2/service_usage.py
Outdated
| @action(methods=['post'], detail=False) | ||
| def post(self, request, *args, **kwargs): | ||
| return self.list(request, *args, **kwargs) | ||
| def retrieve(self, request, pk=None): |
There was a problem hiding this comment.
We'd rather keep DRF signature, i.e. def retrieve(self, request, pk=None, *args, **kwargs).
(for example, format does not work without those parameters)
🤔 Anyway, this endpoint does not seem RESTFul compliant to me. pk is not a unique identifier of an service_usage model.
I'd rather see something like:
/api/v2/organizations/<pk>/service_usage/for organizations/api/v2/service_usage/for users
It may involve more front-end (and back-end) changes but I think it's worth it ;-)
What do you think?
| getUsageForOrganization().then((data) => { | ||
| if (!data) { | ||
| return; | ||
| } | ||
| let lastUpdated: UsageState['lastUpdated'] = null; | ||
| if ('headers' in data && data.headers instanceof Headers) { |
There was a problem hiding this comment.
Could be it simplified with if (data?.headers instanceof Headers)?
There was a problem hiding this comment.
Unfortunately, TS complains here without in acting as a type guard:
kobo/settings/base.py
Outdated
| @@ -1363,6 +1363,9 @@ def dj_stripe_request_callback_method(): | |||
| 'default': env.cache(default='redis://redis_cache:6380/3'), | |||
| } | |||
|
|
|||
| # How long to retain cached responses for kpi endpoints | |||
| ENDPOINT_CACHE_DURATION = env.str('ENDPOINT_CACHE_DURATION', 60 * 15) # 15 minutes | |||
There was a problem hiding this comment.
I think we should better use env.int() instead of env.str() here? Even the default value is an integer?
Checklist
Description
Caches the
api/v2/service_usageendpoint to prevent slowdown on sites with a large number of users/projects. Adds a 'last updated' widget to the Usage page. Allows promotion codes to be used when checking out for a Plan.Notes
ENDPOINT_CACHE_DURATION.service_usageendpoint to use a detail view (/api/v2/organizations/{ORGANIZATION_ID}/service_usage/) instead of POSTing the organization ID.