feat(env): add configurable port overrides via environment variables#683
feat(env): add configurable port overrides via environment variables#683jnun wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
Allow users to override default service ports (NEMOCLAW_GATEWAY_PORT, NEMOCLAW_DASHBOARD_PORT, NEMOCLAW_VLLM_PORT, NEMOCLAW_OLLAMA_PORT) through environment variables. This prevents conflicts when default ports are already in use on the host. New file: - bin/lib/ports.js — central port config with env var overrides and validation (range 1024-65535) Modified files: - bin/nemoclaw.js — use DASHBOARD_PORT for port forwarding - bin/lib/onboard.js — use port constants for gateway, dashboard, inference detection, and Ollama startup - bin/lib/local-inference.js — use VLLM_PORT and OLLAMA_PORT for health checks, base URLs, and error messages - bin/lib/nim.js — use VLLM_PORT for NIM container port mapping - bin/lib/preflight.js — use DASHBOARD_PORT as default check Follows the existing process.env.NEMOCLAW_* pattern used by NEMOCLAW_MODEL, NEMOCLAW_PROVIDER, and NEMOCLAW_GPU. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughCentralizes port configuration by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@bin/lib/ports.js`:
- Around line 9-13: The port validation currently uses parseInt on raw which
allows strings like "8081abc" to pass; update the validation in the function
that parses environment ports (the block creating `parsed` from `raw`) to first
ensure `raw` matches /^\d+$/ (digits-only) and only then convert using
`Number(raw)` (not parseInt), keeping the existing range check (>=1024 &&
<=65535) and throwing the same Error message when validation fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31500cfe-58e2-4ff5-81fc-f8bfee7826e0
📒 Files selected for processing (6)
bin/lib/local-inference.jsbin/lib/nim.jsbin/lib/onboard.jsbin/lib/ports.jsbin/lib/preflight.jsbin/nemoclaw.js
…iables - Tighten parsePort validation: reject non-digit strings (e.g. "8081abc") that parseInt silently accepted; use regex + Number() instead - Add JSDoc to parsePort and all exported constants (docstring coverage) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks for the suggested feature, adding configurable port overrides via environment variables could be really useful for users, I'll make sure to review it carefully. |
Summary
bin/lib/ports.js— central port configuration that readsNEMOCLAW_GATEWAY_PORT,NEMOCLAW_DASHBOARD_PORT,NEMOCLAW_VLLM_PORT, andNEMOCLAW_OLLAMA_PORTfrom environment variables with validation and sensible defaultsprocess.env.NEMOCLAW_*pattern used byNEMOCLAW_MODEL,NEMOCLAW_PROVIDER, andNEMOCLAW_GPUProblem
The default ports (8080, 8000, 18789) conflict with common services (Jenkins, Django, Tomcat, K8s dashboards). Users have no way to change them without editing source code.
Fix
One new file (
bin/lib/ports.js, 23 lines), five surgical edits. No new infrastructure, no.envfile loading, no shell script changes. Environment variables override defaults:Validation rejects invalid values (non-integer, out of range 1024-65535).
Files changed
bin/lib/ports.jsbin/nemoclaw.jsDASHBOARD_PORTbin/lib/onboard.jsbin/lib/local-inference.jsbin/lib/nim.jsbin/lib/preflight.jsTest plan
npx vitest run— 214/219 pass (5 pre-existing failures in install-preflight.test.js)node bin/nemoclaw.js onboardwithNEMOCLAW_GATEWAY_PORT=8081— gateway starts on configured portnode bin/nemoclaw.js --help/--version— CLI smoke testSupersedes #652.
🤖 Generated with Claude Code
Summary by CodeRabbit