feat(frontend): support setting HTTP host via CLI (--http-host) #2523
Conversation
Signed-off-by: Sotetsu Suzugamine <s.suzugamine@gmail.com>
suzusuzu
requested review from
GuanLuo,
biswapanda,
grahamking,
nnshah1,
paulhendricks,
piotrm-nvidia,
ptarasiewiczNV,
rmccorm4,
ryanolson,
tanmayv25 and
tmonty12
as code owners
suzusuzu
requested review from
a team,
PeaBrane,
alec-flowers,
ishandhanani,
jthomson04,
kkranen and
tedzhouhk
as code owners
|
👋 Hi suzusuzu! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
bot
added
the
external-contribution
WalkthroughAdds an optional HTTP host configuration across the stack: new CLI flag and env var parsing, propagation through Python-exposed EntrypointArgs, into LocalModelBuilder/LocalModel, and finally into the HTTP input service to bind the server to a specific host when provided. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as Frontend CLI
participant EP as EntrypointArgs (Py/Rust)
participant LMB as LocalModelBuilder
participant LM as LocalModel
participant HTTP as HTTP Service Builder
User->>CLI: Run with --http-host or DYN_HTTP_HOST
CLI->>EP: Construct EntrypointArgs(http_host=Option<String>)
EP->>LMB: builder.http_host(args.http_host.clone())
LMB->>LM: build() carrying http_host
LM->>HTTP: configure()
alt http_host is Some(host)
HTTP->>HTTP: bind host(host)
note right of HTTP: Bind to specified interface
else None
HTTP->>HTTP: use default host
note right of HTTP: Existing default behavior
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/llm/src/local_model.rs (2)
120-123: Builder setter is fine; consider accepting Into for ergonomicsMinor ergonomics tweak to allow
&strwithout allocation at call sites.Apply:
-pub fn http_host(&mut self, host: Option<String>) -> &mut Self { - self.http_host = host; - self -} +pub fn http_host<S: Into<String>>(&mut self, host: Option<S>) -> &mut Self { + self.http_host = host.map(Into::into); + self +}
334-336: Avoid cloning on getter (optional)Returning an owned
Stringeach time will clone. If the builder chain accepts&str, consider returning a borrowed form.Option A (preferred if feasible at call sites):
- pub fn http_host(&self) -> Option<String> { - self.http_host.clone() - } + pub fn http_host(&self) -> Option<&str> { + self.http_host.as_deref() + }If changing the signature is disruptive, you could also add a secondary accessor, e.g.,
http_host_ref(&self) -> Option<&str>, and migrate call sites incrementally.components/frontend/src/dynamo/frontend/main.py (1)
90-95: Add a light validator and clarify help (optional)
- Users can pass an empty string via shell quoting; that would pass through as an invalid bind. Consider rejecting empty strings.
- The help can clarify that this is the frontend HTTP server host and that IPv6 is supported.
- parser.add_argument( - "--http-host", - type=str, - default=os.environ.get("DYN_HTTP_HOST", "0.0.0.0"), - help="HTTP host for the engine (str). Can be set via DYN_HTTP_HOST env var.", - ) + def _validate_http_host(v: str) -> str: + v = v.strip() + if not v: + raise argparse.ArgumentTypeError("http-host must be a non-empty string") + return v + + parser.add_argument( + "--http-host", + type=_validate_http_host, + default=os.environ.get("DYN_HTTP_HOST", "0.0.0.0"), + help="HTTP host for the frontend server (e.g. 0.0.0.0, 127.0.0.1, ::1). Can also be set via DYN_HTTP_HOST.", + )lib/bindings/python/rust/llm/entrypoint.rs (1)
127-132: Constructor parameter for http_host — consider minimal validation (optional)You may reject empty strings early to avoid confusing runtime errors.
- http_host: Option<String>, + mut http_host: Option<String>, http_port: Option<u16>, @@ - Ok(EntrypointArgs { + if let Some(ref s) = http_host { + if s.trim().is_empty() { + return Err(pyo3::exceptions::PyValueError::new_err( + "http_host must be a non-empty string", + )); + } + } + Ok(EntrypointArgs { engine_type, @@ - http_host, + http_host,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
components/frontend/src/dynamo/frontend/main.py(2 hunks)lib/bindings/python/rust/llm/entrypoint.rs(5 hunks)lib/llm/src/entrypoint/input/http.rs(1 hunks)lib/llm/src/local_model.rs(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
lib/llm/src/entrypoint/input/http.rs (2)
lib/llm/src/local_model.rs (2)
http_host(120-123)http_host(334-336)lib/llm/src/entrypoint.rs (1)
local_model(66-74)
components/frontend/src/dynamo/frontend/main.py (1)
lib/llm/src/local_model.rs (3)
default(65-85)http_host(120-123)http_host(334-336)
lib/bindings/python/rust/llm/entrypoint.rs (1)
lib/llm/src/local_model.rs (11)
router_config(140-143)router_config(350-352)kv_cache_block_size(115-118)http_host(120-123)http_host(334-336)http_port(125-128)http_port(338-340)tls_cert_path(130-133)tls_cert_path(342-344)tls_key_path(135-138)tls_key_path(346-348)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pre-merge-rust (lib/runtime/examples)
- GitHub Check: pre-merge-rust (lib/bindings/python)
- GitHub Check: pre-merge-rust (.)
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (10)
lib/llm/src/local_model.rs (5)
53-55: New http_host field on LocalModelBuilder looks goodOptional host keeps backwards compatibility and lets the service default its bind address when unset.
68-70: Default to None is appropriateLeaving
http_hostasNoneby default is consistent with the builder pattern and avoids surprising binds unless explicitly set by callers.
210-213: Propagation into echo_full path — LGTMEnsures echo engine also honors
http_hostwhen provided.
286-292: Propagation into main LocalModel construction — LGTMCorrectly moves the builder’s
http_hostinto the model instance.
302-308: Storing http_host on LocalModel is consistent with usageNo issues. Field ordering and types are consistent with getters below.
lib/llm/src/entrypoint/input/http.rs (1)
48-50: Bind to provided host when set — LGTMThe conditional host override is correctly applied after TLS/port setup.
components/frontend/src/dynamo/frontend/main.py (1)
218-224: Forwarding http_host to EntrypointArgs — LGTMThe flag is correctly propagated into the engine initialization.
lib/bindings/python/rust/llm/entrypoint.rs (3)
105-110: New http_host in EntrypointArgs struct — LGTMOptional field aligns with the builder design downstream.
116-117: PyO3 constructor signature extended — LGTMAdds
http_host=Nonewithout breaking existing callers.
190-192: Propagation to LocalModelBuilder — LGTM
.http_host(args.http_host.clone())properly threads the value into the builder chain.
|
Thanks @suzusuzu . I was thinking merging port and host into |
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
2 participants
Overview:
This PR introduces a new --http-host option for the dynamo.frontend command line interface, allowing users to configure the HTTP server host address.
Details:
--http-hostargument to dynamo.frontend entrypoint.Where should the reviewer start?
The
--http-hostargument has been added to the Python dynamo.frontend execution:components/frontend/src/dynamo/frontend/main.pyThen, the
--http-hostvalue provided in Python is propagated through Rust:lib/bindings/python/rust/llm/entrypoint.rslib/llm/src/entrypoint/input/http.rslib/llm/src/local_model.rsRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
No related GitHub issue, since this is a minor change.
Summary by CodeRabbit