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

Closed
wants to merge 1 commit into 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.

@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 2 times, most recently 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).

@relyt0925 relyt0925 force-pushed the redis-auth-support branch 2 times, most recently from dff63f8 to 6f26823 Compare June 4, 2025 01:52
@relyt0925 relyt0925 marked this pull request as ready for review June 4, 2025 01:52
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.

Signed-off-by: Tyler Lisowski <[email protected]>
@relyt0925 relyt0925 closed this Jun 4, 2025
@relyt0925 relyt0925 force-pushed the redis-auth-support branch from 6f26823 to 309e16a Compare June 4, 2025 04:12
@relyt0925
Copy link
Author

I'm so sorry @vMaroon I have no idea how I accidentally closed this PR: I reopened a new one at:
#172

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