Repository navigation
feat(router): add --enable-wasm flag to Python CLI and Helm chart - #1729
Conversation
📝 WalkthroughWalkthroughAdds an ChangesWASM enable flag end-to-end
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 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)
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 |
|
Hi @neverCase, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an enable_wasm configuration option to enable WebAssembly module support across the Python bindings and Helm charts. However, a compilation error was identified in bindings/python/src/lib.rs because the enable_wasm parameter is missing from the Router::new function signature, despite being used in the struct initialization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| mesh_port, | ||
| mesh_peer_urls, | ||
| drain_settle_secs, | ||
| enable_wasm, |
There was a problem hiding this comment.
The parameter enable_wasm: bool is missing from the function signature of Router::new (around line 1000). This will cause a compilation error because enable_wasm is not in scope when initializing the Router struct at line 1125, and the PyO3 signature macro will also fail to match the function arguments.
Please update the new function signature in bindings/python/src/lib.rs to include enable_wasm: bool:
fn new(
// ...
drain_settle_secs: u64,
enable_wasm: bool,
) -> PyResult<Self> {![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee67350510
ℹ️ 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".
| mesh_port, | ||
| mesh_peer_urls, | ||
| drain_settle_secs, | ||
| enable_wasm, |
There was a problem hiding this comment.
enable_wasm to the Rust constructor parameters
When building the Python bindings, this initializer now uses enable_wasm, but fn new still has no enable_wasm argument—the parameter list above still ends at drain_settle_secs. In any build of the Python extension with this commit, the Rust compiler has no local named enable_wasm here, so the bindings fail to compile instead of exposing the new CLI flag.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bindings/python/src/lib.rs`:
- Around line 884-885: The pyo3 signature declares `enable_wasm = false` as a
constructor parameter on line 884, but the corresponding `fn new(...)` function
(around lines 891-1000) does not include `enable_wasm` as a parameter in its
function signature, causing it to be undefined when referenced in the
initializer code at lines 1124-1126. Add `enable_wasm` as a parameter to the `fn
new(...)` function signature to match the pyo3 signature declaration, ensuring
the parameter is properly received and available for use in the initialization
logic that references it.
In `@bindings/python/src/smg/router_args.py`:
- Around line 897-901: The `--{prefix}enable-wasm` argument uses `default=False`
which makes the prefixed key always present/non-None, causing the
`from_cli_args()` method to treat it as explicitly set and bypass the fallback
logic for backend args in prefixed mode. Change the `default=False` parameter to
`default=None` in the add_argument call for the enable-wasm flag so that
fallback semantics work correctly when the flag is not explicitly provided by
the user.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0a72645-8c9a-4e1d-9750-e965c3eb9fd7
📒 Files selected for processing (4)
bindings/python/src/lib.rsbindings/python/src/smg/router_args.pydeploy/helm/smg/templates/_helpers.tpldeploy/helm/smg/values.yaml
Signed-off-by: zhujing <1024769485@qq.com>
Signed-off-by: zhujing <1024769485@qq.com>
14ea095 to
5e7f97d
Compare
2 participants
Description
Problem
The --enable-wasm CLI flag was previously only available in the Rust binary entrypoint but missing from the Python launcher (launch_router.py) and Helm chart, causing the WASM module manager to never support posting wasm plugins and the post request always return 500.
Solution
Adds
enable_wasmfield to RouterArgs dataclass + argparse, to thePyO3 Router struct + RouterConfigBuilder chain, and a new
router.wasm.enabledvalue in the Helm chart.Changes
Test Plan
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
enable_wasmoption (default: disabled).enable_wasm.--<prefix>enable-wasmto turn WASM support on when set.router.wasm.enabled), defaulting to disabled for backward compatibility.