Add explicit token-producer backend selector#1582
Conversation
github-actions
Bot
added
the
size/L
Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
04309fb to
cfa3f2f
Compare
Co-authored-by: CodeFree <codefree@chinatelcom.cn> Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
sagearc
left a comment
sagearc
left a comment
There was a problem hiding this comment.
Thanks @zhouyou9505! Looks good to me overall, just left a few minor comments.
There was a problem hiding this comment.
Since {"backend":"estimate","vllm":{}} is now accepted this comment is not longer true
| // Backend selection: `backend` explicitly selects the backend when set. When it | ||
| // is omitted, existing configs continue to infer the backend from `estimate`, | ||
| // `vllm`, `udsTokenizerConfig`, or `modelName`. | ||
| type tokenizerPluginConfig struct { | ||
| // Backend explicitly selects "estimate", "vllm", or "uds". When omitted, | ||
| // the plugin infers the backend from legacy config fields. | ||
| Backend tokenizerBackend `json:"backend,omitempty"` |
There was a problem hiding this comment.
nit: duplicated explanation of the inferred backend path
| // `vllm`, `udsTokenizerConfig`, or `modelName`. | ||
| type tokenizerPluginConfig struct { | ||
| // Backend explicitly selects "estimate", "vllm", or "uds". When omitted, | ||
| // the plugin infers the backend from legacy config fields. |
There was a problem hiding this comment.
The config fields themselves are not legacy, only the implicit inference logic, right?
There was a problem hiding this comment.
Correct. Got it. The fields themselves are not legacy. Only the implicit backend inference behavior is legacy and kept for backward compatibility. I will refine the description.
|
🚨 Unsigned commits detected! Please sign your commits. For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation. |
Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
7d8b3e5 to
69526ed
Compare
|
@sagearc Hi, I’ve addressed the review feedback and pushed the changes. Could you please help review again? Thanks! |
|
Thanks @zhouyou9505 |
2 participants
Summary
backendselector for the token-producer pluginestimate,vllm, andudsbackend selection while preserving legacy inference when omittedFixes #1472
Validation
Local curl smoke testing with a temporary HTTP harness that loads token-producer through
PluginFactory:backend: estimate:POST /v1/completionsreturned estimated token IDs from the in-process backendbackend: vllm:POST /v1/completionswith a local fake vLLM/renderserver returnedtoken_ids: [101,102,103]backend: uds:POST /v1/completionswith a local fake gRPC Unix socket tokenizer returnedtoken_ids: [201,202,203]