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

Update raises ValueError when given an empty dictionary #12

Open
kwzrd opened this issue Mar 14, 2021 · 1 comment
Open

Update raises ValueError when given an empty dictionary #12

kwzrd opened this issue Mar 14, 2021 · 1 comment

Comments

@kwzrd
Copy link

kwzrd commented Mar 14, 2021

Passing an empty dictionary to RedisCache.update produces a ValueError from inside the aioredis dependency.

This is inconsistent with the behaviour of a Python dictionary:

>>> d = {"A": 1}
>>> d.update({})
>>> d
{'A': 1}

If this is intended, it should be documented, as the update method's docstring can be misleading:

This works exactly like dict.update from a normal dictionary.
@kwzrd
Copy link
Author

kwzrd commented Mar 14, 2021

To clarify, I understand that RedisCache doesn't attempt to provide an identical interface to a dictionary anymore, for example update also doesn't take kwargs or no arguments at all. But this behaviour is clearly observable from the function signature.

In comparison, the empty dict behaviour is not visible from looking at the implementation unless one looks into aioredis. It feels natural to assume that update({}) is safe. Perhaps the update could check for this case and exit early?

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

No branches or pull requests

1 participant