-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add redis as cache backend option #1469
Conversation
.circleci/config.yml
Outdated
- run: | ||
name: start redis | ||
command: | | ||
docker run --rm -d -p 6379:6379 redis -m 64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think CI will pass if you remove the -m 64
from the end of this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very sorry it's taken so long to deal with this issue, I'll fix it and resubmit the PR
This is great. With this, to pass tests locally one needs memcached and redis running. I'm thinking that after this is merged, I'll add some environment checks so that the memcached and redis tests are only run if the appropriate service exists. |
Thanks for working on this. You should be able to fix the linting errors by doing |
i get this error like async call, but i'm sure i used sync function of redis. due to i'm not very familiar with CI. maybe i need some help.
|
I think redis doesn't type response from the client aside from a very general type, so I'd use some judicious casts. From your current checkin, I think the following diff would clean up the type and lint issues: |
@nipeone Thank you so much for adding this. I'll make a follow on PR to make it so the redis tests are run conditionally to make it easier to test locally if redis isn't available. |
No description provided.