Repository: Kyria/EsiPy

Issue:
Esipy redis cache cleanup
When using esipy with redis cache a lot of objects are left back. 
Is there a recommended way to clean up old cache objects, or is it safe to delete everything older then x time (like a day)?

Thank you

Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index e9d1fad..9d6d150 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,11 +6,10 @@ services:
   - redis
 
 python:
-  - "2.7"
-  - "3.4"
   - "3.5"
   - "3.6"
   - "3.7"
+  - "3.8"
 
 install:
   - if [[ `python --version | grep 3.6` ]]; then pip install -U setuptools; fi;
diff --git a/esipy/__init__.py b/esipy/__init__.py
index d6319b9..a4e23b2 100644
--- a/esipy/__init__.py
+++ b/esipy/__init__.py
@@ -11,4 +11,4 @@ except ImportError:  # pragma: no cover
     # Not installed or in install (not yet installed) so ignore
     pass
 
-__version__ = '1.1.0'
+__version__ = '1.2.0'
diff --git a/esipy/app.py b/esipy/app.py
index e449cf5..bc00c26 100644
--- a/esipy/app.py
+++ b/esipy/app.py
@@ -103,7 +103,8 @@ class EsiApp(object):
         if res.status_code == 304 and cached_app is not None:
             self.cache.set(
                 cache_key,
-                (cached_app, res.headers, timeout)
+                (cached_app, res.headers, timeout),
+                timeout
             )
             return cached_app
 
@@ -132,7 +133,7 @@ class EsiApp(object):
             )
 
         if self.caching and app:
-            self.cache.set(cache_key, (app, res.headers, timeout))
+            self.cache.set(cache_key, (app, res.headers, timeout), timeout)
 
         return app
 
diff --git a/esipy/cache.py b/esipy/cache.py
index 396f5a8..66b2b35 100644
--- a/esipy/cache.py
+++ b/esipy/cache.py
@@ -2,6 +2,7 @@
 """ Cache objects for EsiPy """
 import hashlib
 import logging
+import datetime
 
 try:
     import pickle
@@ -25,7 +26,7 @@ class BaseCache(object):
     the cache methods used in esipy
     """
 
-    def set(self, key, value):
+    def set(self, key, value, expire=300):
         """ Set a value in the cache. """
         raise NotImplementedError
 
@@ -63,8 +64,9 @@ class FileCache(BaseCache):
         """
         self._cache.close()
 
-    def set(self, key, value):
-        self._cache.set(_hash(key), value)
+    def set(self, key, value, expire=300):
+        expire = None if expire == 0 else expire
+        self._cache.set(_hash(key), value, expire=expire)
 
     def get(self, key, default=None):
         return self._cache.get(_hash(key), default)
@@ -74,7 +76,9 @@ class FileCache(BaseCache):
 
 
 class DictCache(BaseCache):
-    """ BaseCache implementation using Dict to store the cached data. """
+    """ BaseCache implementation using Dict to store the cached data.
+
+    Caution: due to its nature, DictCache do not expire keys !"""
 
     def __init__(self):
         self._dict = {}
@@ -82,12 +86,15 @@ class DictCache(BaseCache):
     def get(self, key, default=None):
         return self._dict.get(key, default)
 
-    def set(self, key, value):
+    def set(self, key, value, expire=300):
         self._dict[key] = value
 
     def invalidate(self, key):
         self._dict.pop(key, None)
 
+    def clear(self):
+        self._dict.clear()
+
 
 class DummyCache(BaseCache):
     """ Base cache implementation that provide a fake cache that
@@ -99,7 +106,7 @@ class DummyCache(BaseCache):
     def get(self, key, default=None):
         return default
 
-    def set(self, key, value):
+    def set(self, key, value, expire=300):
         pass
 
     def invalidate(self, key):
@@ -125,8 +132,9 @@ class MemcachedCache(BaseCache):
         value = self._mc.get(_hash(key))
         return value if value is not None else default
 
-    def set(self, key, value):
-        return self._mc.set(_hash(key), value)
+    def set(self, key, value, expire=300):
+        expire = 0 if expire is None else expire
+        return self._mc.set(_hash(key), value, time=expire)
 
     def invalidate(self, key):
         return self._mc.delete(_hash(key))
@@ -150,8 +158,14 @@ class RedisCache(BaseCache):
         value = self._r.get(_hash(key))
         return pickle.loads(value) if value is not None else default
 
-    def set(self, key, value):
-        return self._r.set(_hash(key), pickle.dumps(value))
+    def set(self, key, value, expire=300):
+        if expire is None or expire == 0:
+            return self._r.set(_hash(key), pickle.dumps(value))
+        return self._r.setex(
+            name=_hash(key),
+            value=pickle.dumps(value),
+            time=datetime.timedelta(seconds=expire),
+        )
 
     def invalidate(self, key):
         return self._r.delete(_hash(key))
diff --git a/esipy/client.py b/esipy/client.py
index 113f48a..06725e0 100644
--- a/esipy/client.py
+++ b/esipy/client.py
@@ -54,7 +54,8 @@ class EsiClient(BaseClient):
             signal to use, instead of using the global API_CALL_STATS
         :param timeout: (optional) default value [None=No timeout]
         timeout in seconds for requests
-
+        :param no_etag_body: (optional) default False, set to return empty
+        response when ETag requests return 304 (normal http behavior)
         """
         super(EsiClient, self).__init__(security)
         self.security = security
@@ -104,6 +105,7 @@ class EsiClient(BaseClient):
         )
 
         self.timeout = kwargs.pop('timeout', None)
+        self.no_etag_body = kwargs.pop('no_etag_body', False)
 
     def _retry_request(self, req_and_resp, _retry=0, **kwargs):
         """Uses self._request in a sane retry loop (for 5xx level errors).
@@ -232,8 +234,8 @@ class EsiClient(BaseClient):
                 raw=six.BytesIO(res.content).getvalue()
             )
 
-        except ValueError:
-            # catch JSONDecodeError/ValueError when response is not JSON
+        except (ValueError, Exception):
+            # catch when response is not JSON
             raise APIException(
                 request.url,
                 res.status_code,
@@ -324,6 +326,7 @@ class EsiClient(BaseClient):
                         content=res.content,
                         url=res.url,
                     ),
+                    cache_timeout
                 )
             else:
                 LOGGER.warning(
@@ -412,7 +415,9 @@ class EsiClient(BaseClient):
 
         # if we have HTTP 304 (content didn't change), return the cached
         # response updated with the new headers
-        if res.status_code == 304 and cached_response is not None:
+        if (res.status_code == 304
+                and cached_response is not None
+                and not self.no_etag_body):
             cached_response.headers['Expires'] = res.headers.get('Expires')
             cached_response.headers['Date'] = res.headers.get('Date')
             return cached_response
diff --git a/esipy/events.py b/esipy/events.py
index d9627f9..32c81fd 100644
--- a/esipy/events.py
+++ b/esipy/events.py
@@ -3,7 +3,6 @@
 to some defined event within EsiPy for the user to be able to do
 specific actions at some times """
 import logging
-import sys
 
 LOGGER = logging.getLogger(__name__)
 
@@ -53,11 +52,11 @@ class Signal(object):
         for receiver in self.event_receivers:
             try:
                 receiver(**kwargs)
-            except Exception as err:  # pylint: disable=W0703
-                if not hasattr(err, '__traceback__'):
-                    LOGGER.error(sys.exc_info()[2])
-                else:
-                    LOGGER.error(getattr(err, '__traceback__'))
+            except Exception:  # pylint: disable=W0703
+                LOGGER.exception(
+                    'Exception while sending to "%s".',
+                    getattr(receiver, '__name__', repr(receiver))
+                )
 
 
 # define required alarms
diff --git a/setup.py b/setup.py
index 4e0c909..598f438 100644
--- a/setup.py
+++ b/setup.py
@@ -1,8 +1,10 @@
-import esipy
+# -*- encoding: utf-8 -*-
+""" Setup for EsiPy """
 import io
-
 from setuptools import setup
 
+import esipy
+
 # install requirements
 install_requirements = [
     "requests",
@@ -39,20 +41,16 @@ setup(
     description='Swagger Client for the ESI API for EVE Online',
     long_description=README,
     install_requires=install_requirements,
-    extras_require={
-        ':python_version == "2.7"': ['futures']
-    },
     tests_require=test_requirements,
     test_suite='nose.collector',
     classifiers=[
         "Development Status :: 5 - Production/Stable",
         "Intended Audience :: Developers",
         "Programming Language :: Python",
-        "Programming Language :: Python :: 2.7",
-        "Programming Language :: Python :: 3.4",
         "Programming Language :: Python :: 3.5",
         "Programming Language :: Python :: 3.6",
         "Programming Language :: Python :: 3.7",
+        "Programming Language :: Python :: 3.8",
         "Programming Language :: Python :: Implementation :: CPython",
     ]
 )


Test patch:
diff --git a/test/test_cache.py b/test/test_cache.py
index c57e7b6..18aac63 100644
--- a/test/test_cache.py
+++ b/test/test_cache.py
@@ -6,6 +6,7 @@ import memcache
 import redis
 import shutil
 import unittest
+import time
 
 from collections import namedtuple
 
@@ -94,6 +95,12 @@ class TestDictCache(BaseTest):
         self.c.invalidate(self.ex_cpx[0])
         self.assertIsNone(self.c.get(self.ex_cpx[0]))
 
+    def test_dict_cache_clear(self):
+        self.assertEqual(self.c._dict[self.ex_str[0]], self.ex_str[1])
+        self.assertEqual(len(self.c._dict), 3)
+        self.c.clear()
+        self.assertEqual(len(self.c._dict), 0)
+
 
 class TestDummyCache(BaseTest):
     """ DummyCache test class. """
@@ -151,6 +158,22 @@ class TestFileCache(BaseTest):
         self.c.invalidate('key')
         self.assertEqual(self.c.get('key'), None)
 
+    def test_file_cache_expire(self):
+        self.c.set('key', 'bar', expire=1)
+        self.assertEqual(self.c.get('key'), 'bar')
+        time.sleep(1)
+        self.assertEqual(self.c.get('key', None), None)
+
+        self.c.set('key', 'bar', expire=0)
+        self.assertEqual(self.c.get('key'), 'bar')
+        time.sleep(1)
+        self.assertEqual(self.c.get('key', None), 'bar')
+
+        self.c.set('foo', 'baz', expire=None)
+        self.assertEqual(self.c.get('foo'), 'baz')
+        time.sleep(1)
+        self.assertEqual(self.c.get('foo', None), 'baz')
+
 
 class TestMemcachedCache(BaseTest):
     """ Memcached tests """
@@ -196,6 +219,22 @@ class TestMemcachedCache(BaseTest):
         with self.assertRaises(TypeError):
             MemcachedCache(None)
 
+    def test_memcached_expire(self):
+        self.c.set(self.ex_str[0], self.ex_str[1], expire=1)
+        self.assertEqual(self.c.get(self.ex_str[0]), self.ex_str[1])
+        time.sleep(1)
+        self.assertEqual(self.c.get(self.ex_str[0], None), None)
+
+        self.c.set(self.ex_str[0], self.ex_str[1], expire=0)
+        self.assertEqual(self.c.get(self.ex_str[0]), self.ex_str[1])
+        time.sleep(1)
+        self.assertEqual(self.c.get(self.ex_str[0], None), self.ex_str[1])
+
+        self.c.set(self.ex_int[0], self.ex_int[1], expire=None)
+        self.assertEqual(self.c.get(self.ex_int[0]), self.ex_int[1])
+        time.sleep(1)
+        self.assertEqual(self.c.get(self.ex_int[0], None), self.ex_int[1])
+
 
 class TestRedisCache(BaseTest):
     """RedisCache tests"""
@@ -239,3 +278,19 @@ class TestRedisCache(BaseTest):
     def test_redis_invalid_argument(self):
         with self.assertRaises(TypeError):
             RedisCache(None)
+
+    def test_redis_expire(self):
+        self.c.set(self.ex_str[0], self.ex_str[1], expire=1)
+        self.assertEqual(self.c.get(self.ex_str[0]), self.ex_str[1])
+        time.sleep(1)
+        self.assertEqual(self.c.get(self.ex_str[0], None), None)
+
+        self.c.set(self.ex_str[0], self.ex_str[1], expire=0)
+        self.assertEqual(self.c.get(self.ex_str[0]), self.ex_str[1])
+        time.sleep(1)
+        self.assertEqual(self.c.get(self.ex_str[0], None), self.ex_str[1])
+
+        self.c.set(self.ex_int[0], self.ex_int[1], expire=None)
+        self.assertEqual(self.c.get(self.ex_int[0]), self.ex_int[1])
+        time.sleep(1)
+        self.assertEqual(self.c.get(self.ex_int[0], None), self.ex_int[1])
diff --git a/test/test_client.py b/test/test_client.py
index 510ca89..5f68026 100644
--- a/test/test_client.py
+++ b/test/test_client.py
@@ -341,6 +341,31 @@ class TestEsiPy(unittest.TestCase):
             res = self.client.request(operation)
             self.assertEqual(res.data.server_version, "1313143")
 
+    def test_esipy_expired_header_etag_no_body(self):
+        # check that the response is empty with no_etag_body=True
+        @httmock.all_requests
+        def check_etag(url, request):
+            return httmock.response(
+                headers={'Etag': '"esipy_test_etag_status"',
+                         'expires': make_expire_time_str(),
+                         'date': make_expire_time_str()},
+                status_code=304)
+
+        operation = self.app.op['get_status']()
+        self.client.no_etag_body = True
+
+        with httmock.HTTMock(eve_status):
+            self.assertEqual(self.cache._dict, {})
+            res = self.client.request(operation)
+            self.assertNotEqual(self.cache._dict, {})
+            self.assertEqual(res.data.server_version, "1313143")
+
+        time.sleep(2)
+
+        with httmock.HTTMock(check_etag):
+            res = self.client.request(operation)
+            self.assertEqual(res.data, None)
+
     def test_esipy_expired_header_noetag(self):
         def check_etag(url, request):
             self.assertNotIn('If-None-Match', request.headers)