Skip to content

[TRTLLM-17271][refactor] Read trtllm-serve environment variables through one typed module - #22041

Draft
JunyiXu-nv wants to merge 26 commits into
NVIDIA:mainfrom
JunyiXu-nv:dev-junyix-feat-serve-typed-env
Draft

JunyiXu-nv wants to merge 26 commits into
NVIDIA:mainfrom
JunyiXu-nv:dev-junyix-feat-serve-typed-env

Conversation

@JunyiXu-nv

Copy link
Copy Markdown
Collaborator

@coderabbitai summary

Description

Stacked on #21992 (Declare trtllm-serve config precedence and server-only options once; it carries #21967 and #21394) and #21406 (Add route, middleware-order and shim-identity snapshots and a CPU test seam for trtllm-serve), joined by one integration merge: the first commits below belong to those PRs and disappear from this diff when they merge; this PR's own change is the last 1 commit. None of them has merged yet, so the edits sit where that base has the code (the disaggregated commands still live in tensorrt_llm/commands/serve.py).

Environment variables are a fourth configuration source next to the YAML, the CLI and --set, invisible in --help and in the serve config schema, and read with ad-hoc parsing wherever they were needed. This PR declares every variable that tensorrt_llm/serve/ (except the published client package serve/scripts/) and the trtllm-serve CLI (tensorrt_llm/commands/serve.py, tensorrt_llm/commands/serve_cli/) read in one typed module, tensorrt_llm/serve/envs.py, and makes every read in those directories go through it:

  • 54 declarations in three blocks: 34 deployment switches (set by whoever runs the server), 18 launcher-internal variables (written by a launcher for a process it starts, or for itself) and 2 external ones (OMPI_COMM_WORLD_RANK, SLURM_PROCID, set by MPI and Slurm). Each has the reader that reproduces the parsing of the call site it replaces, what an unset variable means, a description, the secret flag (TRTLLM_RUNTIME_CONTROL_API_KEY, BRAVE_SEARCH_API_KEY, TAVILY_API_KEY, TLLM_EXECUTOR_ATTACH_INFO) and the files outside these directories that still read os.environ for the same name (also_read_by; those readers are unchanged).
  • Call sites read envs.NAME at the point of use, with the expression shape kept. Values are read from os.environ on every access and never cached: launchers write TRTLLM_RESPONSES_API_DISABLE_STORE, TRTLLM_DISAGG_ROLE and the fleet-worker variables at run time and code reads them later. Variables a launcher passes to exactly one reader (TLLM_FRONTEND_READY_FD, TLLM_FRONTEND_AFFINITY_FD, TLLM_SERVE_LAUNCHER_UDS) are read once and removed with envs.consume().
  • Writes (os.environ[...] = ..., pop as a statement), the launcher enums DisaggLauncherEnvs, DisaggWorkerEnvs and LlmLauncherEnvs, and the whole-environment copy for fleet workers stay where they are; a test ties every enum member and every module-level <NAME>_ENV constant in scope to a declaration, so the lists cannot drift.
  • import os goes where the migrated read was its last use (16 files); COORDINATOR_RESERVATION_TIMEOUT_DEFAULT_S goes because its only use was the read that moved and the default now lives with the declaration (COORDINATOR_RESERVATION_TIMEOUT_ENV and coordinator_reservation_timeout() stay; a test imports them).
  • The lint tests/test_common/serve/env_lint.py rejects direct reads (os.getenv, os.environ.get, os.environ[...], in os.environ, a pop/setdefault whose value is used, also through import os as ... and from os import ...) and from tensorrt_llm.serve.envs import NAME for anything but the module's API, because an imported variable is read once, at import time. Writes and whole-environment copies pass. Its message says what to declare and where; python3 tests/test_common/serve/env_lint.py lists every finding in the scope.
  • tensorrt_llm/serve/AGENTS.md states the rule.
  • Introspection: the module's __getattr__ lets a required variable's KeyError propagate by design, so hasattr(envs, NAME) and inspect.getmembers(envs) raise KeyError while such a variable (for example TRTLLM_DISAGG_CONFIG_FILE) is unset; tools that enumerate the variables use the environment_variables dict instead of attribute access.

Part of the trtllm-serve refactor: CLI and config (epic TRTLLM-17056, task TRTLLM-17271).

Inventory, by an AST scan of every os.getenv, os.environ.get, os.environ[...], in os.environ, os.environ.pop/setdefault/update/copy and iteration over the scope at the base of this PR: 79 lines mentioning os.getenv/os.environ in 19 files; 78 accesses (58 reads, 14 writes, 5 pops, 1 whole-environment copy); 53 distinct variables read or consumed, plus TRTLLM_DISAGG_DEPLOYMENT_ID, which the launcher only writes (usage telemetry reads it) and which is declared so that the whole set is in one place. After: 0 direct reads in the 80 scanned files; the 14 writes, 2 statement pops (plus the pop inside consume()) and the copy remain. One expression-shape change: TRTLLM_DISAGG_SHARED_DRAFT_KV is read once into a local in the MPI worker where the base read it in two expressions; no write intervenes between them, so the outcome is identical for every value, with and without a blocker.

Open PRs that add environment variables in these directories must declare them in envs.py, or the lint fails for whichever lands second: #21808 (TRTLLM_RESPONSES_STREAM_KEEPALIVE_S), #21806 (TRTLLM_RESPONSES_DEFAULT_MAX_OUTPUT_TOKENS), #21735 (TRTLLM_GPT_OSS_LEGACY_CHAT) and the request-trace PR in preparation (TRTLLM_REQUEST_TRACE_DIR, TRTLLM_REQUEST_TRACE_IDENTITY). #22000 moves the disaggregated commands into commands/serve_cli/; whichever of #22000 and this PR lands second re-applies the other's edits to the moved read sites.

Behavior

No behavior change. Every reader reproduces the parsing of the call site it replaces; tests/unittest/serve/test_envs.py::test_readers_reproduce_todays_parsing pins each of the 54 readers in six environment states (unset, empty, 0, 1, text, an integer) against a table written out in the test. The special cases keep their shape: unset and empty differ where they did (TRTLLM_DISAGG_SERVER_DISABLE_GC unset means the collector is off, empty means it is on), TRTLLM_RESPONSES_API_DISABLE_STORE=0 still disables the store (any non-empty value does), the required fleet-worker variables still raise KeyError when missing, and a non-integer TRTLLM_DISAGG_COORDINATOR_PORT or TRTLLM_KVCACHE_AWARE_ROUTER_HASH_TOKENS_PER_BLOCK still raises ValueError at the read. Warnings and validation that a call site does today stay at the call site (anthropic_batches._int_env, web_search._env_int/_env_float, visual_gen_utils.local_media_path_is_disallowed, the web-search unavailability policy), so log text and timing do not change. ENABLE_RESPONSES_DEBUG_MSG is still evaluated once at import.

Moves

None.

CI bookkeeping

  • Multi-GPU table: none. serve/envs.py only declares the variables; the behavior stays in the callers, which the table covers, and a default changed only in envs.py shows up in the parsing test. The PR edits files that are in the table (serve/openai_server.py, responses_utils.py, router_utils.py, openai_client.py, openai_disagg_service.py, serve/render/, serve/extensions/), so the multi-GPU stages run.
  • CODEOWNERS: none.
  • legacy-files.txt: none (serve/envs.py and the test files are new, ruff-formatted files); ruff-legacy-baseline.json: none.
  • Type checking: Pyrefly checks every file under tensorrt_llm/ ([tool.pyrefly] project-includes in pyproject.toml), so the new module needs no registration; scripts/run_pyrefly.sh --strict --no-diff-filter tensorrt_llm/serve/envs.py and the strict changed-lines check of the 19 edited modules report no diagnostics.
  • Test lists: no new entries; tests/unittest/serve/test_envs.py is collected by the unittest/serve entry of l0_cpu.yml and tests/unittest/serve/api/visual_gen/test_envs_lint_visual_gen.py by unittest/serve/api/visual_gen/ (both carry pytest.mark.cpu_only; scripts/test_to_stage_mapping.py maps both entries to the CPU stages).

Test Coverage

New tests (both cpu_only, collected through the directory entries above):

  • tests/unittest/serve/test_envs.py: the lint over the scope finds nothing, and one synthetic case per flagged form and per allowed form proves it sees every read form and ignores writes (an empty scan cannot pass vacuously); the parsing table for all 54 readers; values are read at access time; consume() reads once and rejects names that are not launcher-internal; undeclared names raise AttributeError naming the module; required variables raise KeyError; the launcher enums and the <NAME>_ENV constants in scope are declared; secrets are marked; the TYPE_CHECKING block matches the declarations; also_read_by paths exist and mention the variable; the names the lint lets code import are the module's __all__; AGENTS.md states the rule.
  • tests/unittest/serve/api/visual_gen/test_envs_lint_visual_gen.py: the lint over the VisualGen serving files, which must not scan an empty set.

Local runs on CPU (compiled artifacts of a source build linked in; the GPU stages are left to CI):

  • python3 tests/test_common/serve/env_lint.py: 0 direct environment read(s) in 80 files.
  • pytest -m cpu_only tests/unittest/serve/test_envs.py tests/unittest/serve/api/visual_gen/test_envs_lint_visual_gen.py: 117 passed.
  • Negative check (not committed): with os.getenv("TRTLLM_X") appended to tensorrt_llm/serve/conversation_id.py, test_no_direct_environment_reads fails with tensorrt_llm/serve/conversation_id.py:132 reads the environment directly (os.getenv("TRTLLM_X")). Declare the variable once in tensorrt_llm/serve/envs.py (name, kind deploy/internal/external, default, description, secret) and read it as envs.<NAME>; see tensorrt_llm/serve/AGENTS.md, 'Environment variables'.
  • pytest -m cpu_only tests/unittest/serve: 211 passed, 1 failed: test_no_object_new.py::test_no_new_object_new_sites, whose object.__new__ baseline (from [TRTLLM-17261][test] Add route, middleware-order and shim-identity snapshots and a CPU test seam for trtllm-serve #21406) does not match test_anthropic_routes.py and test_openai_server_graceful_shutdown.py as [None][feat] Anthropic Messages: carry the request id in the message id #20936 and [None][feat] Dense DWDP for Kimi-K3 context servers and a balanced DWDP expert partition #21012 left them on main; it fails identically at the base of this PR, and this PR touches none of its inputs.
  • pytest -m cpu_only tests/unittest/disaggregated: 1791 passed, 5 skipped, 2 xfailed, 4 failed in test_kv_transfer_fail_stop.py, environmental here, not a regression: its child processes start python without the test shim for the compiled artifacts of this host and fail on operator trtllm::swiglu_nvfp4_quantize does not exist; the same tests fail identically at the base of this PR (A/B with the shim on both sides).
  • pytest tests/unittest/api_stability: 99 passed.
  • pytest -m cpu_only tests/unittest/llmapi/test_serve_cli_wiring.py tests/unittest/llmapi/test_serve_cli_net.py tests/unittest/llmapi/test_serve_sleep_mode.py tests/unittest/llmapi/test_disagg_leader_lifecycle.py tests/unittest/_torch/visual_gen/test_visual_gen_utils.py tests/unittest/grpc/openengine/test_frontend_lifecycle.py: 107 passed (test_serve_cli_wiring.py, test_serve_cli_net.py, test_serve_sleep_mode.py, test_visual_gen_utils.py all green); 16 failed in test_disagg_leader_lifecycle.py and test_frontend_lifecycle.py, environmental here, not a regression: its child processes start python without the test shim for the compiled artifacts of this host and fail on operator trtllm::swiglu_nvfp4_quantize does not exist; the same tests fail identically at the base of this PR (A/B with the shim on both sides).
  • pytest tests/unittest/llmapi/test_disagg_telemetry_launcher.py: 13 passed.
  • pytest -m cpu_only tests/unittest/llmapi/apps: 2602 passed; 8 failed in test_render_standalone.py (4), test_serve_lazy_import.py (3) and test_openai_server_graceful_shutdown.py (1), environmental here, not a regression: their child processes start python -m tensorrt_llm.serve.render or python -c "import tensorrt_llm" without the test shim for the compiled artifacts of this host and fail on operator trtllm::swiglu_nvfp4_quantize does not exist; the same 8 fail identically at the base of this PR (A/B with the shim on both sides). The property those two files guard was checked in-process with the shim: importing tensorrt_llm.serve, serve.envs, serve.chat_tokenization and the serve.render modules loads neither server module, and OpenAIServer/OpenAIDisaggServer still load lazily on first access as the real classes.
  • LLM_MODELS_ROOT=... PYTHONPATH=$PWD/tests python3 -m test_common.serve.cpu_tests (every serve CPU test-db entry, one pytest process each, as the CPU stage runs them; 77 entries): 5238 passed, 34 failed in 8 entries, all of them the failures listed above (the ratchet baseline mismatch and the child-process import errors of test_frontend_lifecycle.py, test_disagg_leader_lifecycle.py, test_render_standalone.py, test_serve_lazy_import.py, test_openai_server_graceful_shutdown.py) plus test_metrics.py::test_prometheus_contract and test_grpc_optional.py::test_protobuf_warning_in_fresh_process, which start sys.executable children the same way; every one of these files fails identically at the base of this PR with the shim on both sides. The runner must be given an absolute PYTHONPATH: with the documented relative tests entry every per-entry process is rejected by the repository's sys.path check.
  • pre-commit run --files <every changed file> twice: clean on the second run.
  • Property check of the lint (local, not committed): three Hypothesis properties over generated modules that mix every read form, write form and alias, nested in functions, methods, decorators and blocks (reads flagged exactly, by line and count; alias forms agree with plain os.; envs imports flagged iff a non-API name is imported) hold for 200 generated modules each, and a gate of 19 bugs planted in the lint is caught 19 times out of 19.

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

🤖 Generated with Claude Code

…ller-owned config dicts

On the trtllm-serve config path, launchers and config helpers wrote into
mappings their callers still own: the telemetry opt-out helper, the KV
cache pool provisioning, the HTTP/encoder/embedding launchers and both
gRPC launchers edited the caller's llm_args in place, the shared merge
function popped a key out of the caller's nested telemetry_config
mapping, extract_disagg_cluster_config replaced a sub-mapping inside the
user's YAML, and create_router wrote into the RouterConfig's args dict.
A function that needs a changed mapping now copies first; one whose
caller needs the change returns (or yields) the new mapping and the
caller rebinds. _pop_bool_config_option is the one declared exception:
removing the key from the caller's mapping is the effect both callers
need.

_init_multi_frontend_mode no longer pops num_serve_frontends when
disabled; launch_server, its only enabled=False caller, drops the key
from its own copy instead.

The launcher calls in the serve, mm_embedding_serve and embeddings
subcommands now bind every argument by keyword, so independent edits to
a call site and a signature can no longer be auto-merged into a
positionally misaligned call. Tests that read those calls by position
read the keywords instead.

tests/unittest/llmapi/test_serve_cli_no_mutation.py pins, per rewritten
site, that the caller's mapping is unchanged and that the value handed
downstream is the one handed down before; registered in l0_cpu.yml.

No behavior change.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…apshots and a CPU test seam for trtllm-serve

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… baselines at the merge base

The route reference is generated from the live apps; the object.__new__
baseline and the gRPC import allowance record what exists on main today
and only shrink.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…n the snapshot matrix

An attached frontend of a multi-frontend server registers ten launcher
forwarding routes before the local table; seven share a method and path
with local handlers and three use a path parameter that matches every
batch id, so registration order decides which handler serves them. The
new configuration records all of it, a live invariant pins the
forwarding-before-local order independently of the reference, and the
route-class failure message now says why the msgpack route class matters.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
- AGENTS.md: the CPU test command sets LLM_MODELS_ROOT, which the runner
  requires, and the stage-mapping note says the mapper matches list-entry
  text, so a file listed through its directory maps by the directory.
- check_contracts matches each from-import as <module>.<name>, so
  'from pkg import sub' and 'from .. import sub' count as imports of the
  submodule; new test for both forms and for the allowance entry.
- The VisualGen /v1/models test asserts the 200 status.
- The AGENTS.md command check also recognizes commands that start with
  LLM_MODELS_ROOT=.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…on the current main

The disaggregated frontend now registers its routes with KvHintsRoute
(NVIDIA#20269), which strips kv_hints from request bodies unless the server
trusts its ingress. Regenerate the route reference: the six
disaggregated configurations record KvHintsRoute where they recorded
APIRoute; their route sets and order are unchanged.

The route-class invariant, which a regenerated reference cannot hide,
now requires KvHintsRoute on every API route of the disaggregated
frontend and APIRoute on the coordinator app. Dropping the KV-hint
policy from the frontend fails it even after a regeneration, and both
apps still reject the aggregated server's msgpack route class.

The object.__new__ baseline records the sites on main at the merge base:
test_kv_hints.py builds one server that way (NVIDIA#20269), and
test_harmony_parsing.py no longer builds any (NVIDIA#21176).

The harmony parsing tests are now registered as a whole file (NVIDIA#21411),
so the node-id entry that the CPU-selection test used as its example is
gone. The node-id check now runs select_entries on a one-entry list of
its own, so it no longer depends on which node-id entries l0_cpu.yml
holds.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ail on silent regressions

The route snapshot now requires every configuration other than the three
invalid ones to build, so a configuration that starts failing at construction
cannot pass through a regenerated reference. The object.__new__ ratchet
resolves 'from ... import <server class> as <name>' aliases and records each
baseline site as its enclosing function or class plus the server class, so
swapping one site for another in the same file no longer passes. The baseline
keeps the same 52 sites in the same 26 files.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…r-snapshot-tests

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…r-snapshot-tests

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…r-snapshot-tests

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…r-snapshot-tests

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The embedding server now sizes its encode batcher from the encoder
executor's max_batch_size, max_num_tokens and max_seq_len instead of
reading them from encoder_executor.model_engine. Give
FakeEncoderExecutor the same attributes so the EMBEDDING role builds
again; the real EncoderExecutor no longer has a model_engine.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ig-mutation

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…th the formatters and CODEOWNERS

Create the package tensorrt_llm/commands/serve_cli/, which the trtllm-serve
command line moves into. Its first module, _net.py, receives code from
tensorrt_llm/commands/serve.py, so it is listed with the files the legacy
formatters (yapf, isort, autoflake) keep formatting and the derived lint
configs are regenerated. The package belongs to the runtime team in
CODEOWNERS, like commands/serve.py.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…to commands/serve_cli/_net.py

Move _publish_bound_address, _diagnose_port_in_use, _bind_failure and
_listener_address_family from tensorrt_llm/commands/serve.py into
tensorrt_llm/commands/serve_cli/_net.py unchanged. serve.py imports the
four names back because its listeners still call them; nothing re-exports
them (all four are private). The report-addr test imports the two it uses
from the new module.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… helper with its policy kept

_listener_address_family takes a policy parameter with the three policies
the listeners use today: auto (IPv6 only if the host resolves to IPv6
addresses only; the OpenAI HTTP server and the VisualGen server), ipv4
(always IPv4 without a lookup; the disaggregated server, its coordinator
and its fleet workers, whose reason -- a link-local AAAA record resolves to
fe80:: with scope id 0, which the kernel rejects -- now lives in the
function's docstring) and first (the family of the first getaddrinfo()
result; OpenEngine's pre-start trial bind).

_bind_listener creates the socket, sets SO_REUSEADDR (and SO_REUSEPORT
when several processes serve one port), binds it, and reports a failure
through _bind_failure, closing the socket. The OpenAI HTTP server, the
VisualGen server, the single disaggregated server and the coordinator bind
through it; the fleet worker keeps its own bind (routing it through the
helper would add a port-holder diagnosis and an error log and change its
message) and OpenEngine keeps socket.create_server (it sets IPV6_V6ONLY
and listens, so a trial bind through the helper would detect conflicts
differently). Both take their family from the policy function.

The report-addr test patches the port-holder diagnosis where _bind_failure
looks it up, in the new module.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…d the wiring of the LLM subcommands

test_serve_cli_net.py checks the three address-family policies and the bind
helper with fake sockets, and, for each of the eight listener cases (the
OpenAI HTTP server with one frontend, as the launcher of two and as an
attached frontend; the VisualGen server; the single disaggregated server;
the coordinator; a fleet worker; OpenEngine's trial bind), that the
listener asks for the policy it has today and makes the SO_REUSEPORT choice
it makes today.

test_serve_cli_wiring.py drives trtllm-serve serve, mm_embedding_serve and
embeddings through their click entry points with a distinct value for every
server-side option and checks, by parameter name, what reaches the
launcher, so an option wired to the wrong parameter or a shifted positional
argument fails by name.

Both files run in the CPU stage.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…rt-lock order of the HTTP listener

Three listener tests: a port-0 bind of the OpenAI HTTP server and of the
single disaggregated server publishes the port read back from the bound
socket, not the requested 0; the launcher's port lock is reserved and
entered before the listener bind and left after it, also when the bind
fails. Each pins a line this change moved or re-arranged.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… usage context out of later tests

The disaggregated CLI callback sets the level of the process-wide TRT-LLM
logger and the context of a live usage session before it reaches its
listener. The two tests that run it left the logger at "info" and a live
session's context at "disaggregated" for every later test in the process.
The shared restored_environment fixture now stubs both calls, Logger.set_level
on the class so the undo leaves no attribute on the logger instance, and
set_usage_context in the module that holds the callback; the tests still
exercise only the listener behavior.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…not part of the reviewable change)

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…erver-only options once

The precedence of the --config YAML, the CLI flags and --set was written
once per LLM subcommand, and which keys configure the server rather than
the LLM was written four times: the keys popped out of the YAML, the
reserved --set paths, the options the gRPC server rejects, and the
schema generator's server keys and live objects.

Declare every server-only option once in
tensorrt_llm/commands/serve_cli/_serve_options.py (SERVE_OPTIONS) and
put the precedence in tensorrt_llm/commands/serve_cli/_config_sources.py
(ConfigSources.resolve()). serve, mm_embedding_serve and embeddings
resolve their configuration through it; _RESERVED_PATH_REASONS, the gRPC
rejection check and the schema generator's server keys and live objects
derive from the declarations; the helpers that lost every caller are
deleted. No behavior change: the derived lists are pinned to the
hand-written ones by goldens, and a completeness test ties every Click
option of the three subcommands to an LlmArgs field or a declaration.
The generated schemas are unchanged.

Stacked on the PRs that create commands/serve_cli (NVIDIA#21967) and stop the
launchers from mutating caller-owned config dicts (NVIDIA#21394).

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…all site takes out of the YAML

Review follow-up for the config-source declaration. The unit tests pin
ConfigSources.resolve() per subcommand and the wiring tests pin the
launcher arguments, but nothing tied a call site to the subcommand it
resolves for: a wrong command= on the mm_embedding_serve or embeddings
site would silently swallow a YAML key the LLM should reject. Two
CLI-level cases now pass a server key through --config and check that it
reaches the launcher's arguments (mm_embedding_serve passes
internal_request_auth_key on and takes only allow_request_chat_template
out; embeddings passes allow_request_chat_template on).

Also assert that a top-level --set logs no "Explicit CLI flag(s) ...
override" warning, whether or not the flag was typed: the --set root
leaves the explicit set before the merge, which only the final value
could not show. The module docstring describes the merge function
without narrating what the change left alone.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…VIDIA#21406) into the config-sources branch (integration base, not part of the reviewable change)

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ugh one typed module

tensorrt_llm/serve/envs.py declares every environment variable that
tensorrt_llm/serve/ (except scripts/) and the trtllm-serve CLI read: 54
variables in three blocks (34 deployment switches, 18 launcher-internal
variables, 2 set by MPI or Slurm), each with the reader that reproduces
the parsing of the call site it replaces, its default, a description and
the secret flag. Every in-scope read now goes through envs.NAME, read at
access time; writes, the launcher enums and the whole-environment copy
stay where they are.

tests/test_common/serve/env_lint.py rejects direct os.environ reads in
those directories and value imports from the module (which would freeze
the value at import time); tests/unittest/serve/test_envs.py runs it and
pins every reader's parsing in six environment states, so no default or
parser changes. The VisualGen part of the lint lives under
tests/unittest/serve/api/visual_gen/ so CI's VisualGen narrowing runs it.

Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant