Skip to content

Refactor Redis configuration handling in KV cache scorer #159

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

relyt0925
Copy link

Replace direct Redis address assignment with URL parsing for better configuration management. This ensures proper error handling and compatibility with Redis connection options. Added Redis client dependency to facilitate this change.

Replace direct Redis address assignment with URL parsing for better configuration management. This ensures proper error handling and compatibility with Redis connection options. Added Redis client dependency to facilitate this change.
@relyt0925
Copy link
Author

relyt0925 commented Jun 1, 2025

Depends on: llm-d/llm-d-kv-cache-manager#37

Going to do custom build where I replace current kv-cache-manager and ensure the current strategy of specifying redis Addresses is not broken and I can successfully deploy the inference-scheduler and connect it to a redis deployment with authentication enabled over TLS.

@relyt0925 relyt0925 force-pushed the redis-auth-support branch from 58db67f to 3481388 Compare June 1, 2025 15:56
@relyt0925 relyt0925 force-pushed the redis-auth-support branch from 3481388 to 421b495 Compare June 1, 2025 19:10
@relyt0925
Copy link
Author

relyt0925 commented Jun 1, 2025

I did need to add logic to keep the current path of specifying "hostname:port" still functional without regressions. When that is specified (specifically no protocol prefix is specified): the url string will assume it is redis:// protocol: which is in line with all existing references.

My custom build was successful otherwise.

@vMaroon
Copy link
Member

vMaroon commented Jun 3, 2025

Thank you @relyt0925 - merged your PR and tagged (v0.1.1).

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