-
Notifications
You must be signed in to change notification settings - Fork 690
fix: KeyError in kv_router.py by Initializing active_blocks_dict for New Workers #1827
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
Conversation
|
👋 Hi umang-kedia-hpe! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
WalkthroughThe method Changes
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsexamples/llm/components/kv_router.py (1)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@umang-kedia-hpe thanks for your contribution. This is a good change |
|
Update:
Please let me know if there’s anything else I should address. Thank you for your feedback! |
Overview:
This pull request addresses a potential KeyError in the kv_router.py component by ensuring that the active_blocks_dict is properly initialized for new worker_id entries. This improves the robustness of the router when handling dynamic worker registration and prevents runtime errors during distributed inference.
Details:
Updated the _update_and_get_active_blocks method in kv_router.py to check if a worker_id is present in active_blocks_dict.
If the worker_id is not present, it initializes the entry with the current polled_value for both old_value and predictive_value.
This change prevents KeyError exceptions when new workers are added dynamically or when the router encounters previously unseen worker IDs.
Where should the reviewer start?
Review the changes in examples/llm/components/kv_router.py, specifically the _update_and_get_active_blocks method.
Summary by CodeRabbit