Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Moving from extending Redis made us loose functionality #58

Open
gkorland opened this issue Aug 20, 2020 · 4 comments
Open

Moving from extending Redis made us loose functionality #58

gkorland opened this issue Aug 20, 2020 · 4 comments

Comments

@gkorland
Copy link
Contributor

Moving from extending Redis made us loose functionality ref #57

@dpipemazo
Copy link

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
Copy link
Contributor

ashtul commented Sep 2, 2020

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
Copy link

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
Copy link

zmingee commented Oct 12, 2020

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.

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

No branches or pull requests

4 participants