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

[WJ-1145] Add initial redis support #1668

Merged
merged 29 commits into from
Oct 24, 2023
Merged

[WJ-1145] Add initial redis support #1668

merged 29 commits into from
Oct 24, 2023

Conversation

emmiegit
Copy link
Member

This PR adds Redis support to DEEPWELL, secrets retrieval, a local Redis container (similar to minio) for development, and the GitHub workflows for redis and minio.

I ensure DEEPWELL builds with Redis, and the ping method now checks for the Redis connection in addition to the existing Postgres connection check.

It doesn't fix the redis warning, so I'll just get rid of this.
just add #[derive(Debug)] please, it's not that hard
Permits local connections when hosted in Docker.
Rsmq is single-connection and so cannot be be cloned, which given that
it requires &mut is a problem.

MultiplexedRsmq can be cloned since it has a series of connections
underneath, so it can be cloned, and the &mut requirement in the trait
is not an issue.
Only exists for local, similar to minio for S3.
Since we were missing it and it's another local-only container workflow.
@emmiegit emmiegit self-assigned this Oct 24, 2023
pub localizations: Localizations,
pub mime_analyzer: MimeAnalyzer,
pub job_queue: JobQueue,
pub s3_bucket: Bucket,
}

impl Debug for ServerStateInner {
Copy link
Member Author

Choose a reason for hiding this comment

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

redis::ai::ConnectionManager is not Debug ☹️

Since these builds use local paths only, instead of full-repo context,
this is the correct way to set context for these builds.
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #1668 (cf9a0bf) into develop (fe9f0ee) will decrease coverage by 0.16%.
Report is 14 commits behind head on develop.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1668      +/-   ##
===========================================
- Coverage    40.65%   40.50%   -0.16%     
===========================================
  Files          338      340       +2     
  Lines        10641    10678      +37     
===========================================
- Hits          4326     4325       -1     
- Misses        6315     6353      +38     
Flag Coverage Δ *Carryforward flag
deepwell 1.72% <0.00%> (-0.04%) ⬇️
ftml 76.83% <ø> (ø) Carriedforward from ec5ac7f

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
deepwell/src/main.rs 0.00% <ø> (ø)
deepwell/src/utils/mod.rs 0.00% <ø> (ø)
deepwell/src/config/secrets.rs 0.00% <0.00%> (ø)
deepwell/src/services/error.rs 0.00% <0.00%> (ø)
deepwell/src/utils/debug.rs 0.00% <0.00%> (ø)
deepwell/src/services/context.rs 0.00% <0.00%> (ø)
deepwell/src/redis.rs 0.00% <0.00%> (ø)
deepwell/src/endpoints/misc.rs 0.00% <0.00%> (ø)
deepwell/src/api.rs 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

@emmiegit
Copy link
Member Author

The lint failure is Clippy complaining about unused imports it previously didn't care about. Going to ignore for now since it's unimportant, will fix later.

@emmiegit
Copy link
Member Author

thanks @Yossipossi1

@emmiegit emmiegit merged commit 45c3468 into develop Oct 24, 2023
10 of 11 checks passed
@emmiegit emmiegit deleted the WJ-1145-redis branch October 24, 2023 04:21
@emmiegit emmiegit mentioned this pull request Oct 30, 2023
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.

2 participants