Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for config not updated in cases where it was already created #383

Merged
merged 3 commits into from
Mar 20, 2018
Merged

Fix for config not updated in cases where it was already created #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bea92bf
Fix for config not updated in cases where it was already created
Mar 8, 2018
21af551
Updated docstring
Mar 12, 2018
12e3d20
Improved test
Mar 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions aiocache/factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,13 @@ def set_config(self, config):

All keys in the config are optional, if they are not passed the defaults
for the specified class will be used.

If a config key already exists, it will be updated with the new values.
"""
if "default" not in config:
raise ValueError("default config must be provided")
for config_name in config.keys():
self._caches.pop(config_name, None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update the docstring with this new behavior, it may have some undesired side effects users should be aware. For example if you call multiple times set_config you will be losing the reference to your caches, so we should at least explain it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new test. I'm accessing the private variable _caches though, didn't know how to test it without it.

self._config = config


Expand Down
37 changes: 36 additions & 1 deletion tests/ut/test_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from aiocache import SimpleMemoryCache, RedisCache, caches
from aiocache.factory import _class_from_string, _create_cache
from aiocache.serializers import PickleSerializer
from aiocache.serializers import JsonSerializer, PickleSerializer
from aiocache.plugins import TimingPlugin, HitMissRatioPlugin


Expand Down Expand Up @@ -212,6 +212,41 @@ def test_set_empty_config(self):
with pytest.raises(ValueError):
caches.set_config({})

def test_set_config_updates_existing_values(self):
assert not isinstance(caches.get('default').serializer, JsonSerializer)
caches.set_config({
'default': {
'cache': "aiocache.SimpleMemoryCache",
'serializer': {
'class': "aiocache.serializers.JsonSerializer"
}
}
})
assert isinstance(caches.get('default').serializer, JsonSerializer)

def test_set_config_removes_existing_caches(self):
caches.set_config({
'default': {
'cache': "aiocache.SimpleMemoryCache",
},
'alt': {
'cache': "aiocache.SimpleMemoryCache",
},
})
caches.get('default')
caches.get('alt')
assert len(caches._caches) == 2

caches.set_config({
'default': {
'cache': "aiocache.SimpleMemoryCache",
},
'alt': {
'cache': "aiocache.SimpleMemoryCache",
},
})
assert caches._caches == {}

def test_set_config_no_default(self):
with pytest.raises(ValueError):
caches.set_config({
Expand Down