-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
[RFC] Formalize class-level extension management & consolidate registry-like implementations. #23213
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
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
Signed-off-by: Chenheli Hua <[email protected]>
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
fe7bd47
to
d788bd1
Compare
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
d788bd1
to
7c7e2ac
Compare
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
0329c49
to
e60d6fd
Compare
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
e60d6fd
to
9fefc59
Compare
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.
Please note that this may break existing use cases that use the old extension mechanisms. Small code changes to switch to ExtensionManager is needed for such use cases. However, the change should be very simple (i.e. add a xyz_manager.register(names=[...]) decorator).
why do we need to consolidate these while breaking exsiting code? I'm afraid consolidating them into one file would cause all sorts of problems that need to take care of lazy import and cross-process serialization problems.
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
KV connector factory changes have been removed from this PR. |
Could we please maintain backward compatibility using https://typing.python.org/en/latest/spec/overload.html? You can use https://typing-extensions.readthedocs.io/en/latest/index.html#typing_extensions.deprecated on the overloads so that we can let users know that they should switch to the new names. |
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
8d60a6e
to
f9989a1
Compare
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Signed-off-by: Chenheli Hua <[email protected]>
I reverted the kv connector factory changes since the concern is mainly around lazy imports. The rest are straightforward register/create so can be cleanly migrated. ToolParserManager legacy API is also kept for compatibility. |
I guess by "cross-process serialization issue", Kaichao means ray-related issues (currently vLLM relies on Ray to run cross-node pipeline parallel). |
This pull request has merge conflicts that must be resolved before it can be |
Summary:
There are repeated implementations of registry-like classes that aim to provide extension support for various classes. Here we introduce a
ExtensionManager
as a formal registry for custom extensions so we can consolidate such implementations.Note: KV connector factory has been reverted due to concerns around lazy imports.
Additional registry-like classes would be migrated one by one given that this PR is already huge.
Related to #22932
Test Plan:
pytest tests/plugins/test_extension_manager.py
Model loader + video loader:
Tool call:
Reviewers:
Subscribers:
cc. @robertgshaw2-redhat @yeqcharlotte
Tasks:
Tags:
Purpose
Test Plan
Test Result
(Optional) Documentation Update
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.