Skip to content

WIP: Redis Cluster / TLS support - #316

Draft
sriram1900 wants to merge 1 commit into
codeforstartups:developmentfrom
sriram1900:feat/redis-cluster-tls
Draft

sriram1900 wants to merge 1 commit into
codeforstartups:developmentfrom
sriram1900:feat/redis-cluster-tls

Conversation

@sriram1900

Copy link
Copy Markdown

Description

Work in progress for Redis Cluster / TLS support.

Related issue

Fixes #40

Changes

  • Added tests covering the existing RedisCache client-construction behavior.
  • Verified the Redis cache tests pass.
  • Next: implement Redis Cluster client selection and TLS configuration.
  • Documentation will be updated after implementation.

This PR is currently a draft and is not ready for final review.

@codeforstartups

Copy link
Copy Markdown
Owner

Thanks for starting on this @sriram1900! Holding off on merge since it's marked WIP — a couple of notes to move it forward:

  1. Scope vs title — the PR is titled Redis Cluster / TLS support (Redis Cluster / TLS support #40), but the diff currently only adds two unit tests for existing RedisCache URL/client construction. To close Redis Cluster / TLS support #40 we'd want the actual feature: RedisCluster support and TLS (rediss:// / ssl=True + cert options) in src/dynavec/cache.py, with these tests covering the new paths.
  2. Formatting — there are missing blank lines between the new test functions (e.g. test_redis_cache_uses_url_to_create_client runs straight after the previous function, and before test_redis_cache_stats). That'll trip ruff (E302) in CI — please run make format / ruff check --fix.
  3. Once it's ready, flip it out of draft and I'll re-review. The two tests themselves look like a reasonable start on the injection/URL behavior. 👍

This branch has not been deployed

No deployments
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.

Redis Cluster / TLS support

2 participants