Skip to content

Refactor Redis configuration handling in KV cache scorer#172

Merged
vMaroon merged 2 commits intollm-d:mainfrom
relyt0925:redis-auth-support
Jun 11, 2025
Merged

Refactor Redis configuration handling in KV cache scorer#172
vMaroon merged 2 commits intollm-d:mainfrom
relyt0925:redis-auth-support

Conversation

@relyt0925
Copy link
Contributor

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
Contributor Author

Convo from previous PR that I accidentally closed when fixing lints: #172

@relyt0925 relyt0925 force-pushed the redis-auth-support branch from 309e16a to 85e0107 Compare June 4, 2025 04:18
@relyt0925
Copy link
Contributor Author

@vMaroon for when you have a moment!

Copy link
Member

@vMaroon vMaroon left a comment

Choose a reason for hiding this comment

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

LGTM

@elevran I suggest we accept this as it does make configuration easier from outside. In future iterations of the llm-d-kv-cache-manager, we can further hide the configuration, although I expect that the interaction with the latter will significantly change.

@vMaroon
Copy link
Member

vMaroon commented Jun 5, 2025

@relyt0925 pls update PR base.

@relyt0925 relyt0925 force-pushed the redis-auth-support branch from 85e0107 to 8afc197 Compare June 5, 2025 22:38
@relyt0925
Copy link
Contributor Author

Updated to be based on latest!

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 <lisowski@us.ibm.com>
@relyt0925 relyt0925 force-pushed the redis-auth-support branch from 8afc197 to dc6c327 Compare June 6, 2025 22:49
@relyt0925
Copy link
Contributor Author

@vMaroon reupdated!

@elevran
Copy link
Collaborator

elevran commented Jun 11, 2025

Thanks @relyt0925 - looks good to merge!

@vMaroon vMaroon merged commit 15605fa into llm-d:main Jun 11, 2025
2 checks passed
Jooho pushed a commit to Jooho/llm-d-inference-scheduler that referenced this pull request Sep 30, 2025
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Co-authored-by: konflux-internal-p02[bot] <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
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.

3 participants