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 Thumbor redis storage #54

Merged
merged 3 commits into from
Nov 3, 2022
Merged

Add Thumbor redis storage #54

merged 3 commits into from
Nov 3, 2022

Conversation

guilhermef
Copy link
Member

This storage should be used when using Queued detector on Thumbor

@RaphaelVRossi
Copy link
Member

Hey @guilhermef , maybe we can use Redis client from remotecv.utils to reuse the same code. What do you think?!

@guilhermef
Copy link
Member Author

Hey @RaphaelVRossi, we're using an async Redis client for the storage. Maybe we can the async one on both.

@guilhermef
Copy link
Member Author

@RaphaelVRossi there's also another issue.
The storage is meant to be used as part of Thumbor, instead of remotecv, so the whole config parameters are coming from the context object.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3355783961

  • 0 of 31 (100.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+2.4%) to 62.243%

Files with Coverage Reduction New Missed Lines %
remotecv/image.py 2 86.96%
Totals Coverage Status
Change from base Build 2873778314: 2.4%
Covered Lines: 274
Relevant Lines: 446

💛 - Coveralls

@coveralls
Copy link

coveralls commented Oct 30, 2022

Pull Request Test Coverage Report for Build 3379931166

  • 31 of 31 (100.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+1.8%) to 61.581%

Files with Coverage Reduction New Missed Lines %
remotecv/image.py 2 86.96%
Totals Coverage Status
Change from base Build 2873778314: 1.8%
Covered Lines: 272
Relevant Lines: 443

💛 - Coveralls

Copy link
Member

@scorphus scorphus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Please consider my suggestions/nits below.

.gitignore Outdated Show resolved Hide resolved
remotecv/storages/redis_storage.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
remotecv/storages/redis_storage.py Outdated Show resolved Hide resolved
@codeclimate
Copy link

codeclimate bot commented Nov 2, 2022

Code Climate has analyzed commit b255ba5 and detected 0 issues on this pull request.

View more on Code Climate.

@guilhermef
Copy link
Member Author

Thanks @scorphus

Copy link
Member

@RaphaelVRossi RaphaelVRossi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @guilhermef 🥳

@guilhermef guilhermef merged commit bb074f4 into master Nov 3, 2022
@guilhermef guilhermef deleted the add-redis-storage branch November 3, 2022 12:44
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.

4 participants