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.

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. Add kwargs to plugin methods. #507

Merged
merged 4 commits into from
Oct 19, 2020
Merged

Fix. Add kwargs to plugin methods. #507

merged 4 commits into from
Oct 19, 2020

Conversation

dmtr
Copy link
Contributor

@dmtr dmtr commented Sep 24, 2020

It fixes next problem: if for example namespace parameter is passed to get method, then post_get method raises unexpected keyword argument error.

It fixes problem with post_get raising error when namespace parameter is
passed to get method.
@codecov
Copy link

codecov bot commented Sep 24, 2020

Codecov Report

Merging #507 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #507   +/-   ##
=======================================
  Coverage   99.13%   99.13%           
=======================================
  Files          13       13           
  Lines        1043     1043           
  Branches      116      116           
=======================================
  Hits         1034     1034           
  Misses          9        9           
Impacted Files Coverage Δ
aiocache/plugins.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a966f84...594de09. Read the comment docs.

@argaen
Copy link
Member

argaen commented Oct 16, 2020

Hey @dmtr can you please add a snippet to reproduce this and also tests for the changes?

@dmtr
Copy link
Contributor Author

dmtr commented Oct 16, 2020

Hi!
This code reproduces the bug.

import aiocache
import asyncio

config = {'default': {'cache': 'aiocache.RedisCache', 'endpoint': 'localhost', 'port': 6379, 'timeout': 5, 'serializer': {'class': 'aiocache.serializers.JsonSerializer'}, 'plugins': [{'class': 'aiocache.plugins.HitMissRatioPlugin'}]}}

aiocache.caches.set_config(config)
cache = aiocache.caches.get("default")
loop = asyncio.get_event_loop()

loop.run_until_complete(cache.set("B", 2, namespace="test"))
loop.run_until_complete(cache.get("B", namespace="test"))

It raises error

~/.pyenv/versions/3.7.4/envs/exness/lib/python3.7/site-packages/aiocache/base.py in _plugins(self, *args, **kwargs)
     78             for plugin in self.plugins:
     79                 await getattr(plugin, "post_{}".format(func.__name__))(
---> 80                     self, *args, took=end - start, ret=ret, **kwargs
     81                 )
     82             return ret

TypeError: post_get() got an unexpected keyword argument 'namespace'

@bbelyeu
Copy link
Contributor

bbelyeu commented Oct 18, 2020

Looks good to me. Thanks for the contribution @dmtr !

@pytest.mark.parametrize(
"key,value,namespace", [("a", 1, None), ("b", 2, "test")],
)
async def test_set_and_get(self, memory_cache, key, value, namespace):
Copy link
Member

Choose a reason for hiding this comment

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

the case of not having a namespace is already tested above so we are good for the first case. Let's leave only one test where we pass the namespace explicitly and then we are ready to merge :)

@argaen
Copy link
Member

argaen commented Oct 19, 2020

@dmtr once you've changed the test as I mentioned please remember to do make format before pushing. Otherwise linting will fail (that's why the build is failing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants