Remove aiocache.Cache and alias support #948
Conversation
| cache.multi_get.return_value = [None] | ||
| get_c.side_effect = [cache, None] | ||
| async def test_reuses_cache_instance(self, mock_cache): | ||
| # TODO @review can probably just remove this test. |
There was a problem hiding this comment.
As noted, this test is a bit superfluous now.
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| logger = logging.getLogger(__name__) | |||
|
|
|||
| AIOCACHE_CACHES: Dict[str, Type[BaseCache[Any]]] = {SimpleMemoryCache.NAME: SimpleMemoryCache} | |||
| _AIOCACHE_CACHES: Dict[str, Type[BaseCache[Any]]] = {SimpleMemoryCache.NAME: SimpleMemoryCache} | |||
There was a problem hiding this comment.
I guess the only purpose of this is to load them into __all__ now.
In which case, the SimpleMemory Cache can probably be put in directly, and this can just be initialised to an empty list (there's no point of a dict anymore).
| self, | ||
| ttl=SENTINEL, | ||
| namespace="", | ||
| key_builder=None, | ||
| skip_cache_func=lambda x: False, | ||
| cache=Cache.MEMORY, | ||
| serializer=None, | ||
| plugins=None, | ||
| alias=None, | ||
| cache=None, | ||
| noself=False, |
There was a problem hiding this comment.
Just thinking about this design, maybe we want to allow the cache to be passed as a positional argument (i.e. the common one used in most cases), and everything else should be keyword only. So, like:
self,
cache=None,
*,
ttl=SENTINEL,
...
I'm also wondering if we should make the cache a required argument... Technically, the memory cache has no shoutdown logic, so it doesn't really make a difference (though it'd encourage reusing an instance across multiple decorators).
There was a problem hiding this comment.
Yeah I like the idea of both your points - the default of using memory could be confusing if you expected redis (especially for folks migrating from the old alias stuff).
| self, | ||
| keys_from_attr, | ||
| namespace="", | ||
| key_builder=None, | ||
| skip_cache_func=lambda k, v: False, | ||
| ttl=SENTINEL, | ||
| cache=Cache.MEMORY, | ||
| serializer=None, | ||
| plugins=None, | ||
| alias=None, | ||
| **kwargs, | ||
| cache=None, |
There was a problem hiding this comment.
Same positional/keyword here.
docs/v1_migration.rst
Outdated
There was a problem hiding this comment.
Thanks, that'll be very useful to add to as we get to the final release.
Co-authored-by: Sam Bull <[email protected]>
What do these changes do?
Removes factory.py and associated factory tooling. Updates all the tests and documentation to reflect the changes.
Added a Migration guide to start documenting the changes for a v1 release.
Are there changes in behavior for the user?
Related issue number
Closes #677
Checklist