Repository navigation
feat(python): add --enable-mesh support to Python bindings - #775
Conversation
What changed: - bindings/python/Cargo.toml: add smg-mesh and rand workspace deps - bindings/python/src/lib.rs: add enable_mesh, mesh_server_name, mesh_host, mesh_port, mesh_peer_urls fields to Router struct. Build MeshServerConfig when enable_mesh is true (matching main.rs logic) instead of hardcoding None. - bindings/python/src/smg/router_args.py: add mesh fields to RouterArgs dataclass and --enable-mesh, --mesh-server-name, --mesh-host, --mesh-port, --mesh-peer-urls to argparse CLI args. Why: the Python entrypoint (used in Docker images) lacked mesh server support, causing --enable-mesh to fail with "unrecognized arguments". The Rust binary had this support but the Python bindings did not. Closes #773 Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive support for mesh networking within the Python bindings. It addresses a previous limitation where the Python entrypoint lacked the necessary configuration and argument parsing for mesh server functionality, which led to crashes. By adding the required CLI arguments and integrating the underlying Rust mesh logic, this change enables Python-based deployments to leverage multi-router coordination for enhanced high availability. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds mesh networking support to the Python bindings: workspace dependencies for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as RouterArgs
participant PyRouter as Router (Python)
participant Runtime as Runtime Server
participant Mesh as MeshServerConfig
User->>CLI: pass --enable-mesh + mesh args
CLI->>PyRouter: instantiate Router with mesh params
PyRouter->>PyRouter: validate peer URLs and host:port
alt enable_mesh == true
PyRouter->>Mesh: build MeshServerConfig (self_name, self_addr, init_peer)
PyRouter->>Runtime: attach MeshServerConfig
else
PyRouter->>Runtime: no mesh config
end
Runtime->>Runtime: start server with/without mesh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ea6daf73b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request adds support for mesh networking to the Python bindings, exposing the existing Rust functionality through CLI arguments. The changes in Cargo.toml, lib.rs, and router_args.py are well-aligned with this goal. No specific code review comments were kept after filtering based on repository rules.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bindings/python/src/lib.rs`:
- Around line 1121-1142: When enable_mesh is true the current mesh_server_config
block silently returns None on parse failures; change it to return a Result and
propagate explicit errors instead of disabling mesh: validate parsing of
format!("{}:{}", self.mesh_host, self.mesh_port) and, if it fails, return an Err
with a clear message referencing mesh_host/mesh_port; also validate each entry
of self.mesh_peer_urls by attempting url.parse::<SocketAddr>() and return an Err
if any peer URL is malformed; update the construction of
smg_mesh::MeshServerConfig (self_name, self_addr, init_peer, mtls_config) to be
created on successful parsing and propagate errors up from the function that
currently builds mesh_server_config (use the same function names/fields
enable_mesh, mesh_server_config, mesh_host, mesh_port, mesh_peer_urls, and
smg_mesh::MeshServerConfig).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71589ef1-6d68-49bb-bc01-287fdeac345e
📒 Files selected for processing (3)
bindings/python/Cargo.tomlbindings/python/src/lib.rsbindings/python/src/smg/router_args.py
…y disabling What changed: - bindings/python/src/lib.rs: when --enable-mesh is set, return explicit PyValueError if mesh_host:mesh_port fails to parse as SocketAddr or if a mesh peer URL is malformed. Previously these were silently converted to None, disabling mesh without any signal to the user. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
bfeea85 to
b3498eb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3498eb4ab
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bindings/python/src/lib.rs`:
- Around line 1129-1150: The code currently uses self.mesh_peer_urls.first() and
therefore ignores additional entries; either reject multiple peers or pass them
through—if only a single seed peer is supported, add a guard that checks
self.mesh_peer_urls.len() > 1 and return a pyo3::exceptions::PyValueError naming
the offending list (use the same style as the existing errors), otherwise change
the parsing to map all entries into Vec<std::net::SocketAddr> and set
MeshServerConfig::init_peer accordingly; locate the fix around the
mesh_peer_urls parsing and where MeshServerConfig { init_peer, ... } is
constructed and update the error path or the type plumbing for init_peer to
avoid silently discarding entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a5d17f80-74d6-41ce-9c44-2531e9ae391c
📒 Files selected for processing (1)
bindings/python/src/lib.rs
What changed: - bindings/python/src/lib.rs: move error formatting into the map closure where the URL is already bound, avoiding a redundant .first().unwrap() that clippy flagged. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
…ct#775) Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
…ct#775) Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Summary
--enable-meshand related mesh server CLI args to the Python bindings--enable-meshCloses #773
What changed
smg-meshandrandworkspace dependenciesenable_mesh,mesh_server_name,mesh_host,mesh_port,mesh_peer_urlsfields toRouterstruct. BuildsMeshServerConfigwhenenable_meshis true (same logic asmain.rs).RouterArgsdataclass and CLI argparse group (--enable-mesh,--mesh-server-name,--mesh-host,--mesh-port,--mesh-peer-urls)New CLI args
--enable-meshfalse--mesh-server-name--mesh-host0.0.0.0--mesh-port39527--mesh-peer-urls[]Test plan
cargo check -p smg-pythoncompiles cleansmg launch --enable-meshno longer errors with "unrecognized arguments"smg launch --enable-mesh --mesh-port 40000 --mesh-peer-urls 10.0.0.2:39527passes args correctlySummary by CodeRabbit
New Features
Chores