underyx / flask-redis

A Flask extension for using Redis
Other
436 stars 71 forks source link

Missing magic methods #23

Closed ariscn closed 8 years ago

ariscn commented 8 years ago

With a real Redis instance, I can do redis_store['foo']. With a FlaskRedis instance, this fails.

FlaskRedis wraps Redis by doing:

    def __getattr__(self, name):
        return getattr(self._redis_client, name)

This works for all regular methods, but fails to account for magic methods. According to https://github.com/andymccurdy/redis-py/blob/master/redis/client.py, the supported methods are: __repr__, __delitem__, __getitem__, and __setitem__.

underyx commented 8 years ago

Thanks for the detailed feedback! Do you think we'd be better off using getattribute and writing code to fix normal operation on the class, or should we just explicitly define these magic methods to redirect to the Redis class (risking lack of support for any new magic methods the redis-py project will implement later on)?

On Thu, Oct 22, 2015, 11:08 PM ariscn notifications@github.com wrote:

With a real Redis instance, I can do redis_store['foo']. With a FlaskRedis instance, this fails.

FlaskRedis wraps Redis by doing:

def __getattr__(self, name):
    return getattr(self._redis_client, name)

This works for all regular methods, but fails to account for magic methods. According to https://github.com/andymccurdy/redis-py/blob/master/redis/client.py, the supported methods are: repr delitem, getitem, and setitem.

— Reply to this email directly or view it on GitHub https://github.com/underyx/flask-redis/issues/23.

ariscn commented 8 years ago

The latter is the quickest fix. I literally just finished typing the following:

    def __repr__(self):
        return self._redis_client.__repr__()

    def __getattr__(self, name):
        return getattr(self._redis_client, name)

    def __setitem__(self, name, value):
        return self._redis_client.__setitem__(name, value)

    def __getitem__(self, name):
        return self._redis_client.__getitem__(name)

    def __delitem__(self, name):
        return self._redis_client.__delitem__(name)

I agree that risking breakage (due to changing function signature or new methods) isn't ideal. It looks like the Right Way to do it is a proxy metaclass? http://stackoverflow.com/questions/9057669/how-can-i-intercept-calls-to-pythons-magic-methods-in-new-style-classes

underyx commented 8 years ago

Hey @ariscn! Very, very late (I'm so sorry D:), but I've committed a fix to this. I ended up pretty much just using your code from here, so I've set you as the author of the commit. See here: https://github.com/underyx/flask-redis/commit/92e417de4b7f2a06cad78da0691f0d60e02ea195

I did remove the __repr__ definition though, since it felt wrong to entirely pretend that we're actually the redis.StrictRedis instance.