search

RedisTimeSeries / redistimeseries-py

RedisTimeSeries python client
https://redistimeseries.io
BSD 3-Clause "New" or "Revised" License
99 stars 21 forks source link

Moving from extending Redis made us loose functionality #58

Open gkorland opened 4 years ago

gkorland commented 4 years ago

Moving from extending Redis made us loose functionality ref https://github.com/RedisTimeSeries/redistimeseries-py/pull/57

dpipemazo commented 4 years ago

One thing we observed on this -- I believe that the decision to not inherit the Redis connection caused us to not be able to use the Python Redis Time Series client as the client of choice in our Django codebase using django-redis. We wound up manually implementing the commands we needed and using redis-py so that all of the connection pooling/etc could be taken care of.

ashtul commented 4 years ago

Hi Dan, Thank you for the feedback. We haven't tagged a version yet b/c we have debated what is the best solution. It was done as an effort to allow the user to pass a connection as variable. Any tip you care to share?

dpipemazo commented 4 years ago

Yes, I see and understand the need to pass the connection as a variable. I guess what we found is that anything that previously depended on the redis-py APIs can no longer use redistimeseries-py as a drop-in replacement. The django cache in particular already allows you to swap out the class they use for redis quite easily so this would have been an easy win.

Perhaps you can offer both class types instead of one or the other? I can see use cases for both but the decision makes it harder to utilize things built for redis-py without major code changes. We wound up writing about 50-100 lines of code to implement our own time-series commands using execute_command in redis-py for our django integration since this was easier than having to re-do all of the caching/pooling middleware we got for free and continuing to use redistimeseries-py.

zmingee commented 3 years ago

I've ran into these same issues in my own projects that rely on flask-redis. The extension uses the .from_url() method, and I've tried working around it but ran into the same issues with caching/pooling middlware that @dpipemazo referenced. Using the tagged relase at 0.8.0 is a no-go, due to the bug reported in #60.