-
Notifications
You must be signed in to change notification settings - Fork 313
[Bugfix] fix dynamic config #598
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
[Bugfix] fix dynamic config #598
Conversation
Signed-off-by: Rui Zhang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @zerofishnoodles, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a bug in the dynamic configuration handling by ensuring that the application instance is correctly passed during service discovery reconfiguration and by providing default values for optional parameters in the StaticServiceDiscovery constructor. These changes aim to improve the robustness and flexibility of the dynamic configuration system.
Highlights
- Dynamic Configuration Parameter Passing: I've updated the
reconfigure_service_discoverycalls withinDynamicRouterConfigto explicitly pass theapp=self.appparameter. This ensures that the application instance is correctly provided when reconfiguring both static and Kubernetes service discovery types. - Service Discovery Parameter Defaults: I've added default
NoneorFalsevalues to several optional parameters in the__init__method ofStaticServiceDiscovery. This includesaliases,model_labels,model_types,static_backend_health_checks,prefill_model_labels, anddecode_model_labels, making the constructor more robust and flexible when these optional arguments are not provided.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request fixes a bug in the dynamic configuration update logic by passing the app object during reconfiguration and adding default values for optional parameters in StaticServiceDiscovery. The changes are correct and address the issue for static service discovery. Adding default values to the StaticServiceDiscovery constructor enhances the robustness of the code.
Signed-off-by: Rui Zhang <[email protected]>
* [hotfix] fix dynamic config Signed-off-by: Rui Zhang <[email protected]> * [Bugfix] Add missing field for reconfiguration Signed-off-by: Rui Zhang <[email protected]> --------- Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]>
…concurrency performance (#589) * feat: update requirements to include aiohttp Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: replace HTTPX with aiohttp for asynchronous client handling Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: replace HTTPXClientWrapper with AiohttpClientWrapper for improved async handling Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: replace HTTPXClientWrapper with AiohttpClientWrapper in lifespan management Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: remove httpx from requirements for aiohttp integration Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * refactor: switch from httpx to aiohttp for asynchronous client handling in service discovery Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: add AiohttpClientWrapper for asynchronous HTTP client handling Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: pin aiohttp version to 3.9.5 in requirements Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * refactor: migrate from httpx to aiohttp for asynchronous HTTP requests in request service Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * refactor: update file upload implementation to use aiohttp for asynchronous requests Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: replace httpx with aiohttp in project dependencies and test requirements Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: format dependencies in pyproject.toml for consistency Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: clean up imports and ensure consistent aiohttp version in requirements Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * fix: update aiohttp client closure method for consistency and accuracy in logging Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * fix: correct status code handling in request processing for improved response accuracy Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * bumping version (#593) Signed-off-by: YuhanLiu11 <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Added option to specify priority class (#557) Signed-off-by: Fabijan Marič Vild <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [CI/Build] Change CI runner to L4 (#595) * Add env clean up before run Signed-off-by: Shaoting Feng <[email protected]> * Delete all pods Signed-off-by: Shaoting Feng <[email protected]> * Delete all release Signed-off-by: Shaoting Feng <[email protected]> * test ci runner Signed-off-by: Shaoting <[email protected]> * test ci runner Signed-off-by: Shaoting <[email protected]> * Test again Signed-off-by: Shaoting Feng <[email protected]> * Test again Signed-off-by: Shaoting Feng <[email protected]> * Change python version from 3.12 to 3.10 Signed-off-by: Shaoting Feng <[email protected]> * delete get helm Signed-off-by: Shaoting Feng <[email protected]> * change back python version Signed-off-by: Shaoting Feng <[email protected]> * trigger Signed-off-by: Shaoting Feng <[email protected]> * Add cache: pip Signed-off-by: Shaoting Feng <[email protected]> * Try check-latest: true Signed-off-by: Shaoting Feng <[email protected]> * Changing to 3.10 Signed-off-by: Shaoting Feng <[email protected]> * change to 3.9 Signed-off-by: Shaoting Feng <[email protected]> * Specify 3.9.2 Signed-off-by: Shaoting Feng <[email protected]> * Use conda python Signed-off-by: Shaoting Feng <[email protected]> * Fix vllm without conda Signed-off-by: Shaoting Feng <[email protected]> --------- Signed-off-by: Shaoting Feng <[email protected]> Signed-off-by: Shaoting <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [Bugfix] fix dynamic config (#598) * [hotfix] fix dynamic config Signed-off-by: Rui Zhang <[email protected]> * [Bugfix] Add missing field for reconfiguration Signed-off-by: Rui Zhang <[email protected]> --------- Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [refactor] redesign RST documentation (#592) * initial reorg Signed-off-by: Kobe Chen <[email protected]> * pass CICD Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * fix: update helm.rst Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * Sharing KV Cache Across Instances Signed-off-by: Kobe Chen <[email protected]> * docker Signed-off-by: Kobe Chen <[email protected]> * community meeting Signed-off-by: Kobe Chen <[email protected]> * add deployment 3 options Signed-off-by: Kobe Chen <[email protected]> * remove A: Signed-off-by: Kobe Chen <[email protected]> * docker Signed-off-by: Kobe Chen <[email protected]> --------- Signed-off-by: Kobe Chen <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [Misc] revert uv.lock (#604) Signed-off-by: Kobe Chen <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * fix: update response content iteration method for improved performance Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Replace httpx with aiohttp for prefill and decode clients in K8sServiceNameServiceDiscovery, updating timeout handling accordingly. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Update file upload function to include file parameter in aiohttp post request Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Refactor response handling in request service to return JSON data and correctly set response status code in sleep/wakeup routing. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Add initialization of aiohttp ClientSession in service discovery during app startup - Implemented method in , , and classes to set up aiohttp ClientSession for prefill and decode endpoints. - Updated function in to call if available, ensuring proper session management during application startup. - Adjusted logging statements in to reference the attribute of the client sessions for accurate logging of routing requests. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Refactor whitespace and formatting in app.py for improved readability Signed-off-by: Ifta Khairul Alam Adil <[email protected]> --------- Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> Signed-off-by: YuhanLiu11 <[email protected]> Signed-off-by: Fabijan Marič Vild <[email protected]> Signed-off-by: Shaoting Feng <[email protected]> Signed-off-by: Shaoting <[email protected]> Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Kobe Chen <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Co-authored-by: Fabhiahn <[email protected]> Co-authored-by: Shaoting <[email protected]> Co-authored-by: Rui Zhang <[email protected]> Co-authored-by: Kobe Chen <[email protected]>
* [hotfix] fix dynamic config Signed-off-by: Rui Zhang <[email protected]> * [Bugfix] Add missing field for reconfiguration Signed-off-by: Rui Zhang <[email protected]> --------- Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: [email protected] <[email protected]>
…concurrency performance (vllm-project#589) * feat: update requirements to include aiohttp Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: replace HTTPX with aiohttp for asynchronous client handling Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: replace HTTPXClientWrapper with AiohttpClientWrapper for improved async handling Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: replace HTTPXClientWrapper with AiohttpClientWrapper in lifespan management Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: remove httpx from requirements for aiohttp integration Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * refactor: switch from httpx to aiohttp for asynchronous client handling in service discovery Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * feat: add AiohttpClientWrapper for asynchronous HTTP client handling Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: pin aiohttp version to 3.9.5 in requirements Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * refactor: migrate from httpx to aiohttp for asynchronous HTTP requests in request service Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * refactor: update file upload implementation to use aiohttp for asynchronous requests Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: replace httpx with aiohttp in project dependencies and test requirements Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: format dependencies in pyproject.toml for consistency Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * chore: clean up imports and ensure consistent aiohttp version in requirements Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * fix: update aiohttp client closure method for consistency and accuracy in logging Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * fix: correct status code handling in request processing for improved response accuracy Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * bumping version (vllm-project#593) Signed-off-by: YuhanLiu11 <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Added option to specify priority class (vllm-project#557) Signed-off-by: Fabijan Marič Vild <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [CI/Build] Change CI runner to L4 (vllm-project#595) * Add env clean up before run Signed-off-by: Shaoting Feng <[email protected]> * Delete all pods Signed-off-by: Shaoting Feng <[email protected]> * Delete all release Signed-off-by: Shaoting Feng <[email protected]> * test ci runner Signed-off-by: Shaoting <[email protected]> * test ci runner Signed-off-by: Shaoting <[email protected]> * Test again Signed-off-by: Shaoting Feng <[email protected]> * Test again Signed-off-by: Shaoting Feng <[email protected]> * Change python version from 3.12 to 3.10 Signed-off-by: Shaoting Feng <[email protected]> * delete get helm Signed-off-by: Shaoting Feng <[email protected]> * change back python version Signed-off-by: Shaoting Feng <[email protected]> * trigger Signed-off-by: Shaoting Feng <[email protected]> * Add cache: pip Signed-off-by: Shaoting Feng <[email protected]> * Try check-latest: true Signed-off-by: Shaoting Feng <[email protected]> * Changing to 3.10 Signed-off-by: Shaoting Feng <[email protected]> * change to 3.9 Signed-off-by: Shaoting Feng <[email protected]> * Specify 3.9.2 Signed-off-by: Shaoting Feng <[email protected]> * Use conda python Signed-off-by: Shaoting Feng <[email protected]> * Fix vllm without conda Signed-off-by: Shaoting Feng <[email protected]> --------- Signed-off-by: Shaoting Feng <[email protected]> Signed-off-by: Shaoting <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [Bugfix] fix dynamic config (vllm-project#598) * [hotfix] fix dynamic config Signed-off-by: Rui Zhang <[email protected]> * [Bugfix] Add missing field for reconfiguration Signed-off-by: Rui Zhang <[email protected]> --------- Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [refactor] redesign RST documentation (vllm-project#592) * initial reorg Signed-off-by: Kobe Chen <[email protected]> * pass CICD Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * fix: update helm.rst Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * fix Signed-off-by: Kobe Chen <[email protected]> * Sharing KV Cache Across Instances Signed-off-by: Kobe Chen <[email protected]> * docker Signed-off-by: Kobe Chen <[email protected]> * community meeting Signed-off-by: Kobe Chen <[email protected]> * add deployment 3 options Signed-off-by: Kobe Chen <[email protected]> * remove A: Signed-off-by: Kobe Chen <[email protected]> * docker Signed-off-by: Kobe Chen <[email protected]> --------- Signed-off-by: Kobe Chen <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * [Misc] revert uv.lock (vllm-project#604) Signed-off-by: Kobe Chen <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * fix: update response content iteration method for improved performance Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Replace httpx with aiohttp for prefill and decode clients in K8sServiceNameServiceDiscovery, updating timeout handling accordingly. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Update file upload function to include file parameter in aiohttp post request Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Refactor response handling in request service to return JSON data and correctly set response status code in sleep/wakeup routing. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Add initialization of aiohttp ClientSession in service discovery during app startup - Implemented method in , , and classes to set up aiohttp ClientSession for prefill and decode endpoints. - Updated function in to call if available, ensuring proper session management during application startup. - Adjusted logging statements in to reference the attribute of the client sessions for accurate logging of routing requests. Signed-off-by: Ifta Khairul Alam Adil <[email protected]> * Refactor whitespace and formatting in app.py for improved readability Signed-off-by: Ifta Khairul Alam Adil <[email protected]> --------- Signed-off-by: Ifta khairul Alam Adil <[email protected]> Signed-off-by: Ifta Khairul Alam Adil <[email protected]> Signed-off-by: YuhanLiu11 <[email protected]> Signed-off-by: Fabijan Marič Vild <[email protected]> Signed-off-by: Shaoting Feng <[email protected]> Signed-off-by: Shaoting <[email protected]> Signed-off-by: Rui Zhang <[email protected]> Signed-off-by: Kobe Chen <[email protected]> Co-authored-by: Yuhan Liu <[email protected]> Co-authored-by: Fabhiahn <[email protected]> Co-authored-by: Shaoting <[email protected]> Co-authored-by: Rui Zhang <[email protected]> Co-authored-by: Kobe Chen <[email protected]> Signed-off-by: [email protected] <[email protected]>
Fix dynamic config
FIX #596 (link existing issues this PR will resolve)
BEFORE SUBMITTING, PLEASE READ THE CHECKLIST BELOW AND FILL IN THE DESCRIPTION ABOVE
-swhen doinggit commit[Bugfix],[Feat], and[CI].Detailed Checklist (Click to Expand)
Thank you for your contribution to production-stack! Before submitting the pull request, please ensure the PR meets the following criteria. This helps us maintain the code quality and improve the efficiency of the review process.
PR Title and Classification
Please try to classify PRs for easy understanding of the type of changes. The PR title is prefixed appropriately to indicate the type of change. Please use one of the following:
[Bugfix]for bug fixes.[CI/Build]for build or continuous integration improvements.[Doc]for documentation fixes and improvements.[Feat]for new features in the cluster (e.g., autoscaling, disaggregated prefill, etc.).[Router]for changes to thevllm_router(e.g., routing algorithm, router observability, etc.).[Misc]for PRs that do not fit the above categories. Please use this sparingly.Note: If the PR spans more than one category, please include all relevant prefixes.
Code Quality
The PR need to meet the following code quality standards:
pre-committo format your code. SeeREADME.mdfor installation.DCO and Signed-off-by
When contributing changes to this project, you must agree to the DCO. Commits must include a
Signed-off-by:header which certifies agreement with the terms of the DCO.Using
-swithgit commitwill automatically add this header.What to Expect for the Reviews
We aim to address all PRs in a timely manner. If no one reviews your PR within 5 days, please @-mention one of YuhanLiu11
, Shaoting-Feng or ApostaC.