From 8852267e99b8dd2dcbd83127531280b818481762 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Sat, 22 Dec 2018 01:04:59 +0100 Subject: [PATCH] atos-genesys: do not use threads (#29320) As threads were hiding errors from asynchronous requests, we now still hide errors when cache is full, but log them. --- passerelle/apps/atos_genesys/models.py | 6 ++++-- passerelle/apps/atos_genesys/utils.py | 23 ++++++++++------------- tests/test_atos_genesys.py | 2 +- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/passerelle/apps/atos_genesys/models.py b/passerelle/apps/atos_genesys/models.py index eff5841..230d6d0 100644 --- a/passerelle/apps/atos_genesys/models.py +++ b/passerelle/apps/atos_genesys/models.py @@ -97,7 +97,8 @@ class Resource(BaseResource, HTTPResource): cache = utils.RowLockedCache( function=self.call_select_codifications, row=self, - key_prefix='atos-genesys-codifications') + key_prefix='atos-genesys-codifications', + logger=self.logger) return cache() @endpoint(name='codifications', @@ -274,7 +275,8 @@ class Resource(BaseResource, HTTPResource): cache = utils.RowLockedCache( function=self.call_select_usager, row=link, - key_prefix='atos-genesys-usager') + key_prefix='atos-genesys-usager', + logger=self.logger) dossier = cache(link.id_per) # build text as "id_per - prenom - no text_parts = [str(link.id_per), '-'] diff --git a/passerelle/apps/atos_genesys/utils.py b/passerelle/apps/atos_genesys/utils.py index c2122c9..cced33e 100644 --- a/passerelle/apps/atos_genesys/utils.py +++ b/passerelle/apps/atos_genesys/utils.py @@ -1,11 +1,12 @@ import time import six -import threading from contextlib import contextmanager from django.db import transaction from django.core.cache import cache +from passerelle.utils.jsonresponse import APIError + DEFAULT_DURATION = 5 * 60 # 5 minutes # keep data in cache for 1 day, i.e. we can answer a request from cache for 1 day @@ -29,11 +30,12 @@ class RowLockedCache(object): a thread, prevent multiple update using row locks on database models and an update cache key. ''' - def __init__(self, function, row=None, duration=DEFAULT_DURATION, key_prefix=None): + def __init__(self, function, logger=None, row=None, duration=DEFAULT_DURATION, key_prefix=None): self.function = function self.row = row self.duration = duration self.key_prefix = key_prefix or function.__name__ + self.logger = logger def _key(self, *args, **kwargs): keys = [] @@ -59,17 +61,12 @@ class RowLockedCache(object): cacheline = cache.get(key) if now - timestamp < self.duration: return cacheline['value'] - updatekey = 'update-' + key - update = cache.get(updatekey) - if not update or (update['timestamp'] - now >= self.duration): - cache.set(updatekey, {'timestamp': now}, CACHE_DURATION) - - def update(): - value = self.function(*args, **kwargs) - cache.set(key, {'value': value, 'timestamp': now}, CACHE_DURATION) - cache.delete(updatekey) - - threading.Thread(target=update).start() + try: + value = self.function(*args, **kwargs) + cache.set(key, {'value': value, 'timestamp': now}, CACHE_DURATION) + except APIError as e: + if self.logger: + self.logger.error('failure to update cache %s', e) return cacheline['value'] else: value = self.function(*args, **kwargs) diff --git a/tests/test_atos_genesys.py b/tests/test_atos_genesys.py index 757817d..b0e6360 100644 --- a/tests/test_atos_genesys.py +++ b/tests/test_atos_genesys.py @@ -215,7 +215,7 @@ def test_row_locked_cache(genesys, freezer): assert rlc() == 1 assert f.calls == 1 - # Check that cache update only launch one thread and f() is called only once again + # Check that with cache update f() is called only once again freezer.move_to('2018-01-01 00:02:00') F.value = 2 counter = 0 -- 2.18.0