feat: remove --connector flag for vLLM backend (LLM-90) - #6450
Merged
Conversation
github-actions
Bot
added
feat
documentation
Contributor
Contributor
WalkthroughThis pull request deprecates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/backends/vllm/launch/disagg_lmcache.sh (1)
18-24:⚠️ Potential issue | 🟡 MinorCorrect the missing LMCache environment variables in the prefill worker invocation.
The JSON schema for
kv_connector_extra_config.connectorsis correct and validated byPdConnector. However, the prefill worker should set LMCache-specific environment variables that control memory management and CPU offloading, which are present in other LMCache examples:
LMCACHE_CHUNK_SIZELMCACHE_LOCAL_CPULMCACHE_MAX_LOCAL_CPU_SIZESee
tests/lmcache/deploy-lmcache_enabled-dynamo-disag.shfor the pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/backends/vllm/launch/disagg_lmcache.sh` around lines 18 - 24, The prefill worker invocation in disagg_lmcache.sh is missing LMCache environment variables; set LMCACHE_CHUNK_SIZE, LMCACHE_LOCAL_CPU, and LMCACHE_MAX_LOCAL_CPU_SIZE alongside the existing DYN_VLLM_KV_EVENT_PORT / VLLM_NIXL_SIDE_CHANNEL_PORT / CUDA_VISIBLE_DEVICES before calling python3 -m dynamo.vllm (the prefill-worker invocation), following the same pattern and value choices used in tests/lmcache/deploy-lmcache_enabled-dynamo-disag.sh so LMCache memory management and CPU offloading are enabled for the prefill worker.docs/pages/components/kvbm/kvbm-guide.md (1)
444-448:⚠️ Potential issue | 🟡 MinorStale
--connector kvbmreference missed in the nsys profiling example.Line 447 still uses the deprecated flag and will emit a
FutureWarningwhen users follow this example:python -m dynamo.vllm --model Qwen/Qwen3-0.6B --connector kvbm📝 Proposed fix
-python -m dynamo.vllm --model Qwen/Qwen3-0.6B --connector kvbm +python -m dynamo.vllm --model Qwen/Qwen3-0.6B --kv-transfer-config '{"kv_connector":"DynamoConnector","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_role":"kv_both"}'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pages/components/kvbm/kvbm-guide.md` around lines 444 - 448, The example invocation in the docs still uses the deprecated CLI flag "--connector kvbm" in the "python -m dynamo.vllm --model Qwen/Qwen3-0.6B --connector kvbm" example; remove that deprecated flag from the nsys profiling example (leave "python -m dynamo.vllm --model Qwen/Qwen3-0.6B") or replace it with the current, documented CLI flag for KVBM if one exists per the CLI reference; update the example line in kvbm-guide.md accordingly so it no longer emits the FutureWarning.
🧹 Nitpick comments (6)
examples/backends/vllm/launch/disagg_kvbm_2p2d.sh (1)
28-41: Consider extracting the repeated prefill--kv-transfer-configinto a shell variable.Lines 28 and 40 contain an identical 275-character JSON string. Any future edit (connector options, module path, etc.) must be applied to both independently — a typical copy/paste drift risk.
♻️ Proposed refactor — deduplicate via a variable
+# Shared kv-transfer-config for all prefill workers (KVBM PdConnector) +PREFILL_KV_CFG='{"kv_connector":"PdConnector","kv_role":"kv_both","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_connector_extra_config":{"connectors":[{"kv_connector":"DynamoConnector","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_role":"kv_both"},{"kv_connector":"NixlConnector","kv_role":"kv_both"}]}}' + DYN_VLLM_KV_EVENT_PORT=20082 \ VLLM_NIXL_SIDE_CHANNEL_PORT=20098 \ DYN_KVBM_CPU_CACHE_GB=20 \ CUDA_VISIBLE_DEVICES=2 \ python3 -m dynamo.vllm \ --model Qwen/Qwen3-0.6B \ --is-prefill-worker \ - --kv-transfer-config '{"kv_connector":"PdConnector","kv_role":"kv_both","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_connector_extra_config":{"connectors":[{"kv_connector":"DynamoConnector","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_role":"kv_both"},{"kv_connector":"NixlConnector","kv_role":"kv_both"}]}}' \ + --kv-transfer-config "$PREFILL_KV_CFG" \ --enforce-eager & DYN_VLLM_KV_EVENT_PORT=20083 \ VLLM_NIXL_SIDE_CHANNEL_PORT=20099 \ DYN_KVBM_LEADER_ZMQ_PUB_PORT=56003 \ DYN_KVBM_LEADER_ZMQ_ACK_PORT=56004 \ DYN_KVBM_CPU_CACHE_GB=20 \ CUDA_VISIBLE_DEVICES=3 \ python3 -m dynamo.vllm \ --model Qwen/Qwen3-0.6B \ --is-prefill-worker \ - --kv-transfer-config '{"kv_connector":"PdConnector","kv_role":"kv_both","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_connector_extra_config":{"connectors":[{"kv_connector":"DynamoConnector","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_role":"kv_both"},{"kv_connector":"NixlConnector","kv_role":"kv_both"}]}}' \ + --kv-transfer-config "$PREFILL_KV_CFG" \ --enforce-eager🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/backends/vllm/launch/disagg_kvbm_2p2d.sh` around lines 28 - 41, The long JSON passed to --kv-transfer-config is duplicated; extract it into a shell variable (e.g., KV_TRANSFER_CONFIG) and reference that variable in both places to avoid drift: assign the full JSON string to KV_TRANSFER_CONFIG (ensuring proper single- or double-quoting/escaping) and replace both explicit --kv-transfer-config '...json...' usages with --kv-transfer-config "$KV_TRANSFER_CONFIG" (used in the backgrounded command and the python3 invocation), so updates only need to be made in one place.examples/backends/vllm/launch/disagg_lmcache.sh (1)
22-24: Consider breaking the 300-character JSON blob into a variable for readability.The entire
--kv-transfer-configpayload is on one line, making both code review and future edits error-prone.♻️ Suggested refactor
+PREFILL_KV_CONFIG='{"kv_connector":"PdConnector","kv_role":"kv_both","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_connector_extra_config":{"connectors":[{"kv_connector":"LMCacheConnectorV1","kv_role":"kv_both"},{"kv_connector":"NixlConnector","kv_role":"kv_both"}]}}' + DYN_VLLM_KV_EVENT_PORT=20081 \ VLLM_NIXL_SIDE_CHANNEL_PORT=20097 \ CUDA_VISIBLE_DEVICES=1 \ python3 -m dynamo.vllm \ --model Qwen/Qwen3-0.6B \ --is-prefill-worker \ - --kv-transfer-config '{"kv_connector":"PdConnector","kv_role":"kv_both","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_connector_extra_config":{"connectors":[{"kv_connector":"LMCacheConnectorV1","kv_role":"kv_both"},{"kv_connector":"NixlConnector","kv_role":"kv_both"}]}}' + --kv-transfer-config "${PREFILL_KV_CONFIG}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/backends/vllm/launch/disagg_lmcache.sh` around lines 22 - 24, Move the long JSON passed to --kv-transfer-config into a named shell variable to improve readability and reduce line length; extract the payload containing "kv_connector":"PdConnector", "kv_role":"kv_both", "kv_connector_module_path":"kvbm.vllm_integration.connector" and the connectors array with "LMCacheConnectorV1" and "NixlConnector" into a multi-line heredoc or quoted variable (e.g., KV_TRANSFER_CONFIG) and then reference it in the launch command as --kv-transfer-config "$KV_TRANSFER_CONFIG" alongside the existing flags (--model Qwen/Qwen3-0.6B and --is-prefill-worker).components/src/dynamo/vllm/tests/test_vllm_unit.py (1)
278-314: Missing no-warning test;test_connector_to_kv_transfer_json_multiassertion is underspecified.Two gaps:
The PR objective explicitly lists "Using
--kv-transfer-configdirectly: no warning" as a tested path, but there is no test for it.
test_connector_to_kv_transfer_json_multionly asserts that theconnectorskey exists inkv_connector_extra_config, without verifying the actual entries (count, connector names, or module paths).✅ Suggested additions
def test_kv_transfer_config_direct_no_warning(mock_vllm_cli): """Test that --kv-transfer-config used directly emits no FutureWarning.""" mock_vllm_cli( "--model", "Qwen/Qwen3-0.6B", "--kv-transfer-config", '{"kv_connector":"NixlConnector","kv_role":"kv_both"}', "--enforce-eager", ) with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") parse_args() future_warnings = [x for x in w if issubclass(x.category, FutureWarning)] assert len(future_warnings) == 0, ( f"Expected no FutureWarning when --kv-transfer-config used directly, got: " f"{[str(x.message) for x in future_warnings]}" )For the multi-connector test, strengthen the assertion:
def test_connector_to_kv_transfer_json_multi(): """Test _connector_to_kv_transfer_json for multiple connectors uses PdConnector.""" from dynamo.vllm.args import _connector_to_kv_transfer_json result = _connector_to_kv_transfer_json(["kvbm", "nixl"]) parsed = json.loads(result) assert parsed["kv_connector"] == "PdConnector" - assert "connectors" in parsed["kv_connector_extra_config"] + connectors = parsed["kv_connector_extra_config"]["connectors"] + assert len(connectors) == 2 + connector_names = {c["kv_connector"] for c in connectors} + assert connector_names == {"DynamoConnector", "NixlConnector"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/src/dynamo/vllm/tests/test_vllm_unit.py` around lines 278 - 314, The tests lack coverage for the "no warning when using --kv-transfer-config directly" path and the multi-connector assertion is too weak: add a new unit test (e.g., test_kv_transfer_config_direct_no_warning) that uses mock_vllm_cli with "--kv-transfer-config" and calls parse_args() while capturing warnings to assert there are zero FutureWarning instances; and update test_connector_to_kv_transfer_json_multi to assert the actual connectors list contents (count, expected connector names like "kvbm" and "nixl", and their module paths such as "kvbm.vllm_integration.connector" or "nixl" mapping) from the JSON returned by _connector_to_kv_transfer_json so the test verifies exact entries rather than only the presence of the "connectors" key.components/src/dynamo/vllm/args.py (1)
376-411: Duplicated connector→config mapping across_connector_to_kv_transfer_jsonandcreate_kv_transfer_config.Both functions maintain identical
lmcache/nixl/kvbm→ config-dict mappings and the same single-vs-multi branching logic. If a connector mapping changes, both must be updated in lockstep. Consider extracting a shared_build_connector_dicts(connector_list) -> List[dict]helper and reusing it in both functions.Also applies to: 454-481
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/src/dynamo/vllm/args.py` around lines 376 - 411, The mapping logic for connectors is duplicated between _connector_to_kv_transfer_json and create_kv_transfer_config; extract that shared mapping into a new helper function (e.g. _build_connector_dicts(connector_list) -> List[dict]) that returns the list of connector dicts for "lmcache", "nixl", "kvbm" (including the kv_connector_module_path for kvbm), then update _connector_to_kv_transfer_json and create_kv_transfer_config to call _build_connector_dicts and keep their single-vs-multi branching logic (single -> json of first dict; multi -> PdConnector wrapper) so updates only need to be made in one place.tests/kvbm_integration/test_consolidator_router_e2e.py (1)
922-922: Useless conditional — both branches produce the same value.wait_time = 5 if engine == "trtllm" else 5This simplifies to
wait_time = 5with no behavioral difference. Likely a placeholder left over from an earlier draft where the values differed.♻️ Proposed fix
- wait_time = 5 if engine == "trtllm" else 5 + wait_time = 5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/kvbm_integration/test_consolidator_router_e2e.py` at line 922, The conditional assigning wait_time is redundant because both branches return 5; replace the ternary expression in the test (variable wait_time) with a direct assignment wait_time = 5, or if different wait durations were intended for specific engine values, update the conditional to use the correct alternate value for the engine (referencing the engine variable and the wait_time assignment in test_consolidator_router_e2e.py).tests/kvbm_integration/test_determinism_disagg.py (1)
162-163: Consider extracting the repeated complex JSON strings to named constants.The long inline JSON on this line (and its duplicates in
disagg_kvbm.shanddisagg_kvbm.yaml) must all be updated in sync whenever the connector configuration changes. Extracting them to module-level constants (e.g.,_VLLM_DECODER_KV_CONFIG,_VLLM_PREFILL_KV_CONFIG,_TRTLLM_PREFILL_KV_CONFIG) in the test file, and referencing them in the commands, would make future edits safer and improve readability.♻️ Suggested extraction
+# KV transfer configuration constants +_VLLM_DECODER_KV_CONFIG = ( + '{"kv_connector":"NixlConnector","kv_role":"kv_both"}' +) +_VLLM_PREFILL_KV_CONFIG = ( + '{"kv_connector":"PdConnector","kv_role":"kv_both",' + '"kv_connector_module_path":"kvbm.vllm_integration.connector",' + '"kv_connector_extra_config":{"connectors":[' + '{"kv_connector":"DynamoConnector","kv_connector_module_path":"kvbm.vllm_integration.connector","kv_role":"kv_both"},' + '{"kv_connector":"NixlConnector","kv_role":"kv_both"}]}}' +) +_TRTLLM_PREFILL_KV_CONFIG = ( + '{"kv_connector":"DynamoConnector",' + '"kv_connector_module_path":"kvbm.vllm_integration.connector",' + '"kv_role":"kv_both"}' +)Then replace the raw strings at the call sites:
- "--kv-transfer-config", - '{"kv_connector":"NixlConnector","kv_role":"kv_both"}', + "--kv-transfer-config", + _VLLM_DECODER_KV_CONFIG,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/kvbm_integration/test_determinism_disagg.py` around lines 162 - 163, The long inline JSON connector config should be extracted to module-level constants (e.g., _VLLM_DECODER_KV_CONFIG, _VLLM_PREFILL_KV_CONFIG, _TRTLLM_PREFILL_KV_CONFIG) in tests/kvbm_integration/test_determinism_disagg.py and the test command arguments should reference those constants instead of repeating raw JSON; update the occurrences of the "--kv-transfer-config" argument (and any other duplicated JSON args in this file) to use the named constants, keep the constants as exact JSON strings used by the CLI, and ensure any other call sites (disagg_kvbm.sh, disagg_kvbm.yaml) are updated to use the same single source of truth if possible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/src/dynamo/vllm/args.py`:
- Around line 113-117: The check for whether the connector was explicitly set
uses "--connector" in sys.argv which misses "--connector=value" forms; update
the logic that sets _connector_explicitly_set on dynamo_config to detect both
the separate flag and the equals syntax by scanning sys.argv (e.g., any(a ==
"--connector" or a.startswith("--connector=") for a in sys.argv)) and still
treat "DYN_CONNECTOR" in os.environ as explicit.
In `@components/src/dynamo/vllm/tests/test_vllm_unit.py`:
- Around line 203-236: Rename test functions and docstrings that mention
"DeprecationWarning" to "FutureWarning" (e.g.,
test_connector_nixl_emits_deprecation_warning ->
test_connector_nixl_emits_future_warning and
test_connector_none_emits_deprecation_warning ->
test_connector_none_emits_future_warning) and update their docstrings
accordingly; also update any assertion messages that say "DeprecationWarning"
(for example the message in the assertion inside
test_connector_nixl_emits_deprecation_warning that currently reads "Expected at
least one DeprecationWarning") to reference "FutureWarning" so the test names,
docstrings, and assertion messages consistently reflect the actual check
(issubclass(x.category, FutureWarning)) performed by parse_args() and the tests.
In `@examples/backends/vllm/launch/agg_request_planes.sh`:
- Line 49: Restore the explicit connector flag in the aggregated-mode launch
command so the script invokes dynamo.vllm with "--connector none" (e.g., change
the call currently invoking "python -m dynamo.vllm --model Qwen/Qwen3-0.6B
--enforce-eager") to avoid implicitly enabling the nixl default; alternatively,
if you prefer keeping the flag removed, add a one-line inline comment next to
the invocation explaining that the FutureWarning about implicit nixl is expected
and harmless in aggregated mode — update the same pattern in the related scripts
(agg.sh and agg_multimodal.sh) to keep behavior consistent.
In `@tests/kvbm_integration/test_determinism_disagg.py`:
- Around line 239-241: The TRT-LLM prefiller is passing the vLLM-only flag
`--kv-transfer-config` which TRT-LLM's argparse (DynamoRuntimeArgGroup /
DynamoTrtllmArgGroup) does not recognize; remove `--kv-transfer-config` from the
TRT-LLM prefiller invocation (the list around the prefiller/decoder setup) or
replace it with TRT-LLM-supported args (e.g., use the
`--connector`/`--connector-config` option defined in DynamoRuntimeArgGroup) and
ensure the TRT-LLM decoder argument list mirrors the intended config handling
used by the vLLM decoder (or implements connector-based KV transfer) so both
prefiller and decoder use compatible flags.
---
Outside diff comments:
In `@docs/pages/components/kvbm/kvbm-guide.md`:
- Around line 444-448: The example invocation in the docs still uses the
deprecated CLI flag "--connector kvbm" in the "python -m dynamo.vllm --model
Qwen/Qwen3-0.6B --connector kvbm" example; remove that deprecated flag from the
nsys profiling example (leave "python -m dynamo.vllm --model Qwen/Qwen3-0.6B")
or replace it with the current, documented CLI flag for KVBM if one exists per
the CLI reference; update the example line in kvbm-guide.md accordingly so it no
longer emits the FutureWarning.
In `@examples/backends/vllm/launch/disagg_lmcache.sh`:
- Around line 18-24: The prefill worker invocation in disagg_lmcache.sh is
missing LMCache environment variables; set LMCACHE_CHUNK_SIZE,
LMCACHE_LOCAL_CPU, and LMCACHE_MAX_LOCAL_CPU_SIZE alongside the existing
DYN_VLLM_KV_EVENT_PORT / VLLM_NIXL_SIDE_CHANNEL_PORT / CUDA_VISIBLE_DEVICES
before calling python3 -m dynamo.vllm (the prefill-worker invocation), following
the same pattern and value choices used in
tests/lmcache/deploy-lmcache_enabled-dynamo-disag.sh so LMCache memory
management and CPU offloading are enabled for the prefill worker.
---
Duplicate comments:
In `@examples/backends/vllm/launch/agg_multimodal.sh`:
- Line 71: The aggregated multimodal launcher removed the explicit connector
override causing the nixl default to activate; restore the previous behavior by
adding the explicit connector flag back to the vllm invocation in
agg_multimodal.sh (the python -m dynamo.vllm command) — include "--connector
none" alongside existing flags (e.g., --enable-multimodal, --model $MODEL_NAME,
$MODEL_SPECIFIC_ARGS, "${EXTRA_ARGS[@]}") so the nixl default is not
unintentionally used.
In `@examples/backends/vllm/launch/agg.sh`:
- Line 32: The launch command in agg.sh (the python -m dynamo.vllm invocation)
inadvertently dropped the --connector none flag (same regression as in
agg_request_planes.sh), changing runtime behavior; restore the original behavior
by re-adding the --connector none argument to the command line or, if
intentional, explicitly document and implement the new default connector
behavior in the dynamo.vllm entrypoint so the script's behavior remains
unchanged—look for the python -m dynamo.vllm call in agg.sh and either append
--connector none to the "${EXTRA_ARGS[@]}" or update the vllm startup code to
explicitly default to "none".
---
Nitpick comments:
In `@components/src/dynamo/vllm/args.py`:
- Around line 376-411: The mapping logic for connectors is duplicated between
_connector_to_kv_transfer_json and create_kv_transfer_config; extract that
shared mapping into a new helper function (e.g.
_build_connector_dicts(connector_list) -> List[dict]) that returns the list of
connector dicts for "lmcache", "nixl", "kvbm" (including the
kv_connector_module_path for kvbm), then update _connector_to_kv_transfer_json
and create_kv_transfer_config to call _build_connector_dicts and keep their
single-vs-multi branching logic (single -> json of first dict; multi ->
PdConnector wrapper) so updates only need to be made in one place.
In `@components/src/dynamo/vllm/tests/test_vllm_unit.py`:
- Around line 278-314: The tests lack coverage for the "no warning when using
--kv-transfer-config directly" path and the multi-connector assertion is too
weak: add a new unit test (e.g., test_kv_transfer_config_direct_no_warning) that
uses mock_vllm_cli with "--kv-transfer-config" and calls parse_args() while
capturing warnings to assert there are zero FutureWarning instances; and update
test_connector_to_kv_transfer_json_multi to assert the actual connectors list
contents (count, expected connector names like "kvbm" and "nixl", and their
module paths such as "kvbm.vllm_integration.connector" or "nixl" mapping) from
the JSON returned by _connector_to_kv_transfer_json so the test verifies exact
entries rather than only the presence of the "connectors" key.
In `@examples/backends/vllm/launch/disagg_kvbm_2p2d.sh`:
- Around line 28-41: The long JSON passed to --kv-transfer-config is duplicated;
extract it into a shell variable (e.g., KV_TRANSFER_CONFIG) and reference that
variable in both places to avoid drift: assign the full JSON string to
KV_TRANSFER_CONFIG (ensuring proper single- or double-quoting/escaping) and
replace both explicit --kv-transfer-config '...json...' usages with
--kv-transfer-config "$KV_TRANSFER_CONFIG" (used in the backgrounded command and
the python3 invocation), so updates only need to be made in one place.
In `@examples/backends/vllm/launch/disagg_lmcache.sh`:
- Around line 22-24: Move the long JSON passed to --kv-transfer-config into a
named shell variable to improve readability and reduce line length; extract the
payload containing "kv_connector":"PdConnector", "kv_role":"kv_both",
"kv_connector_module_path":"kvbm.vllm_integration.connector" and the connectors
array with "LMCacheConnectorV1" and "NixlConnector" into a multi-line heredoc or
quoted variable (e.g., KV_TRANSFER_CONFIG) and then reference it in the launch
command as --kv-transfer-config "$KV_TRANSFER_CONFIG" alongside the existing
flags (--model Qwen/Qwen3-0.6B and --is-prefill-worker).
In `@tests/kvbm_integration/test_consolidator_router_e2e.py`:
- Line 922: The conditional assigning wait_time is redundant because both
branches return 5; replace the ternary expression in the test (variable
wait_time) with a direct assignment wait_time = 5, or if different wait
durations were intended for specific engine values, update the conditional to
use the correct alternate value for the engine (referencing the engine variable
and the wait_time assignment in test_consolidator_router_e2e.py).
In `@tests/kvbm_integration/test_determinism_disagg.py`:
- Around line 162-163: The long inline JSON connector config should be extracted
to module-level constants (e.g., _VLLM_DECODER_KV_CONFIG,
_VLLM_PREFILL_KV_CONFIG, _TRTLLM_PREFILL_KV_CONFIG) in
tests/kvbm_integration/test_determinism_disagg.py and the test command arguments
should reference those constants instead of repeating raw JSON; update the
occurrences of the "--kv-transfer-config" argument (and any other duplicated
JSON args in this file) to use the named constants, keep the constants as exact
JSON strings used by the CLI, and ensure any other call sites (disagg_kvbm.sh,
disagg_kvbm.yaml) are updated to use the same single source of truth if
possible.
tmonty12
approved these changes
Feb 20, 2026
Contributor
|
There are still edge cases that depends on the dynamo/components/src/dynamo/vllm/args.py Line 286 in 2e52920 Also when settings the distributed executer backend |
alec-flowers
force-pushed
the
feat/LLM-90-deprecate-connector
branch
from
February 22, 2026 22:57
2e52920 to
0baaec8
Compare
alec-flowers
force-pushed
the
feat/LLM-90-deprecate-connector
branch
from
February 22, 2026 23:02
0baaec8 to
c832114
Compare
PeaBrane
approved these changes
Feb 24, 2026
Hard-remove the --connector flag for the vLLM backend. Users must now use --kv-transfer-config with explicit JSON instead. The default connector is changed from ["nixl"] to [] so aggregated serving works without any connector flag. TRT-LLM connector support is unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
Signed-off-by: alec-flowers <aflowers@nvidia.com>
Signed-off-by: alec-flowers <aflowers@nvidia.com>
- Remove UniProcExecutor GIL contention workaround (fixed upstream in vllm-project/vllm#29476) - Fix _uses_nixl_connector() and _uses_dynamo_connector() to detect connectors nested inside PdConnector's kv_connector_extra_config Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
alec-flowers
force-pushed
the
feat/LLM-90-deprecate-connector
branch
from
February 24, 2026 02:58
78d47e9 to
3e7812d
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alec-flowers <aflowers@nvidia.com>
yao531441
pushed a commit
to yao531441/dynamo
that referenced
this pull request
May 13, 2026
Signed-off-by: alec-flowers <aflowers@nvidia.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--connectorflag for the vLLM backend — using it now raises aValueErrorwith a migration hint showing the equivalent--kv-transfer-configJSON["nixl"]to[]so aggregated serving works without any connector flag--kv-transfer-configwhen using--is-prefill-worker(no more implicit nixl)--connectorto--kv-transfer-configTest plan
PYTHONPATH=... python3 -m pytest -xvv components/src/dynamo/vllm/tests/test_vllm_unit.py— new error tests passpython3 -m pytest -xvv components/src/dynamo/trtllm/tests/test_trtllm_unit.py— TRT-LLM unaffectedgrep -r '\-\-connector' examples/ tests/ docs/shows zero vLLM-related hits (only TRT-LLM remains)aggregated_lmcacheandaggregated_lmcache_multiprocCI tests pass (root cause: default["nixl"]conflicted with--kv-transfer-config)Closes LLM-90
Closes LLM-95
🤖 Generated with Claude Code