Added ttl parameter in cache class #405
Conversation
Codecov Report
@@ Coverage Diff @@
## master #405 +/- ##
==========================================
+ Coverage 99.76% 99.76% +<.01%
==========================================
Files 9 9
Lines 861 866 +5
Branches 91 91
==========================================
+ Hits 859 864 +5
Misses 2 2
Continue to review full report at Codecov.
|
aiocache/base.py
Outdated
| @@ -99,9 +103,10 @@ class BaseCache: | |||
|
|
|||
| def __init__( | |||
| self, serializer=None, plugins=None, | |||
| namespace=None, key_builder=None, timeout=5): | |||
| namespace=None, ttl=None, key_builder=None, timeout=5): | |||
There was a problem hiding this comment.
Can you add it as the last attribute? Otherwise change could be breaking if someone is not naming the attributes when calling
aiocache/base.py
Outdated
| @@ -9,6 +9,8 @@ | |||
|
|
|||
| logger = logging.getLogger(__file__) | |||
|
|
|||
| sentinel = object() | |||
There was a problem hiding this comment.
Lets put it uppercase. Since its used in other modules, I prefer this to be more visible
aiocache/decorators.py
Outdated
| await self.cache.set(key, value, ttl=self.ttl) | ||
| kwargs = {} | ||
| if self.ttl is not sentinel: | ||
| kwargs['ttl'] = self.ttl |
There was a problem hiding this comment.
why do we need to replicate the logic here? This is already done in the class so if we just propagate self.ttl here (having sentinel as default value) it should be enough right?
There was a problem hiding this comment.
I wanted to avoid having to update all the tests to use SENTINEL when mocking the call. Updated.
aiocache/decorators.py
Outdated
| @@ -291,8 +295,11 @@ def get_cache_keys(self, f, args, kwargs): | |||
|
|
|||
| async def set_in_cache(self, result, fn_args, fn_kwargs): | |||
| try: | |||
| kwargs = {} | |||
| if self.ttl is not sentinel: | |||
| kwargs['ttl'] = self.ttl | |||
tests/ut/test_base.py
Outdated
| await base_cache.add(pytest.KEY, "value") | ||
|
|
||
| assert base_cache._add.call_count == 1 | ||
| assert base_cache._add.call_args[1]['ttl'] == 10 |
There was a problem hiding this comment.
Lets use assert_called_once_with, more compact
tests/ut/test_base.py
Outdated
| def set_test_ttl(self, base_cache): | ||
| base_cache.ttl = 10 | ||
| yield | ||
| base_cache.ttl = None |
There was a problem hiding this comment.
You don't need to reset the ttl to None every time. The base_cache fixture is per test so this fixture is not needed, just set the ttl of base_cache directly in the test whenever you need it
tests/ut/test_base.py
Outdated
| await base_cache.add(pytest.KEY, "value", ttl=None) | ||
|
|
||
| assert base_cache._add.call_count == 1 | ||
| assert base_cache._add.call_args[1]['ttl'] is None |
There was a problem hiding this comment.
I think we are missing the case where we don't pass ttl explicitly and its not set either at he class too
As discussed in #387
ttlwas the only parameter not allowed by default in the cache class. Caches instances that have this parameter in the constructor will use it by default in all the calls, unless a specific parameter is passed.