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

Add redis as cache backend option #1469

Merged
merged 11 commits into from
May 24, 2024
Merged

Add redis as cache backend option #1469

merged 11 commits into from
May 24, 2024

Conversation

nipeone
Copy link
Contributor

@nipeone nipeone commented Feb 25, 2024

No description provided.

- run:
name: start redis
command: |
docker run --rm -d -p 6379:6379 redis -m 64
Copy link
Member

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.

Copy link
Contributor Author

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

@manthey
Copy link
Member

manthey commented Apr 1, 2024

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.

@manthey
Copy link
Member

manthey commented May 23, 2024

Thanks for working on this. You should be able to fix the linting errors by doing tox -e format.

@nipeone
Copy link
Contributor Author

nipeone commented May 23, 2024

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.

large_image/cache_util/rediscache.py:60: error: Argument 1 to "len" has incompatible type "Union[Awaitable[Any], Any]"; expected "Sized"  [arg-type]
large_image/cache_util/rediscache.py:65: error: Incompatible return value type (got "Union[Awaitable[Any], Any]", expected "bool")  [return-value]
large_image/cache_util/rediscache.py:79: error: Argument 1 to "loads" has incompatible type "Union[Awaitable[Any], Any]"; expected "Buffer"  [arg-type]
large_image/cache_util/rediscache.py:114: error: Incompatible return value type (got "Union[Awaitable[Any], Any]", expected "int")  [return-value]
large_image/cache_util/rediscache.py:143: error: Value of type "Union[Awaitable[Any], Any]" is not indexable  [index]
large_image/cache_util/rediscache.py:151: error: List or tuple expected as variadic arguments  [misc]

@manthey
Copy link
Member

manthey commented May 23, 2024

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:
diff.txt

@manthey
Copy link
Member

manthey commented May 24, 2024

@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.

@manthey manthey merged commit 12b0595 into girder:master May 24, 2024
14 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants