-
Notifications
You must be signed in to change notification settings - Fork 676
docs: change docs to default port 8000 #2876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: PeaBrane <yanrpei@gmail.com>
WalkthroughDocumentation-only updates change the example/default HTTP port for the OpenAI-compatible frontend and related references from 8080 to 8000 across READMEs, guides, diagrams, and curl examples. No code, APIs, or behavior modified. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/lmcache/README.md (1)
82-93: Fix invalid JSON in curl payload (unquoted model).The value of "model" must be a JSON string; the current example will fail.
- "model": Qwen/Qwen3-0.6B, + "model": "Qwen/Qwen3-0.6B",Port update to 8000 is correct here.
docs/architecture/dynamo_flow.md (1)
18-26: Update all remaining HTTP port references from 8080 to 8000.
Occurrences found in:
- launch/dynamo-run/src/main.rs (USAGE: default
--http-port 8080)- tests/lmcache/* scripts (assume
localhost:8080)- docs/_includes/quick_start_local.rst (
curl localhost:8080/v1/chat/completions)- docs/guides/dynamo_deploy/installation_guide.md (
modelExpressURL=http://...:8080)- deploy/cloud/operator (cmd flags, manager_auth_proxy_patch.yaml, internal tests)
- deploy/cloud/helm/platform (values.yaml, operator component, README)
- deploy/metrics/prometheus.yml (frontend scrape targets, demo instructions)
- container/launch_message.txt (
python -m dynamo.frontend [--http-port 8080])
🧹 Nitpick comments (13)
docs/guides/dynamo_deploy/create_deployment.md (1)
91-91: Use consistent capitalization and command style for the Frontend launch.Earlier in the doc you use “Frontend” (one word) and
python -m .... Suggest aligning here and formatting as code.-The front end is launched with "python3 -m dynamo.frontend [--http-port 8000] [--router-mode kv]" +The Frontend is launched with: + +`python -m dynamo.frontend [--http-port 8000] [--router-mode kv]`components/backends/vllm/deepseek-r1.md (2)
29-41: Prefer explicit scheme in curl URL.Using
http://is clearer and copy-paste friendly across environments.-curl localhost:8000/v1/chat/completions \ +curl http://localhost:8000/v1/chat/completions \
34-37: Clean up typos in the sample prompt.Minor misspellings (“ests”, “familt”) in the example text; fixing improves polish.
docs/support_matrix.md (1)
88-88: Show explicit Docker port mappings to avoid confusion.Adding a concrete mapping example helps users replace
--network hostsafely.-... by mapping only the necessary ports (e.g., 4222 for nats, 2379/2380 for etcd, 8000 for frontend). +... by mapping only the necessary ports (e.g., `-p 4222:4222 -p 2379:2379 -p 2380:2380 -p 8000:8000`).docs/guides/dynamo_run.md (2)
75-76: Use explicit scheme in curl.-curl localhost:8000/v1/models +curl http://localhost:8000/v1/models
80-81: Use explicit scheme in curl.-... http://localhost:8000/v1/chat/completions +... http://localhost:8000/v1/chat/completionslib/runtime/examples/system_metrics/README.md (1)
188-206: Optional: add a one-liner to verify both endpoints are up.Small usability boost for readers.
Then make curl requests to the frontend (see the [main README](../../../../README.md)) + +Quick checks: +`curl -sSf http://localhost:8000/metrics >/dev/null && echo "Frontend metrics OK"` +`curl -sSf http://localhost:8081/metrics >/dev/null && echo "System metrics OK"`tests/lmcache/README.md (2)
63-67: Add a language hint to fenced block.Specify a language to satisfy MD040 and improve rendering of the ASCII diagram.
-``` +```text HTTP Request → Dynamo Ingress(8000) → Dynamo Worker → Direct Inference Environment: ENABLE_LMCACHE=0--- `70-76`: **Add a language hint to fenced block.** Ditto for the LMCache diagram. ```diff -``` +```text HTTP Request → Dynamo Ingress(8000) → Dynamo Worker → LMCache-enabled Inference Environment: ENABLE_LMCACHE=1 LMCACHE_CHUNK_SIZE=256 LMCACHE_LOCAL_CPU=True LMCACHE_MAX_LOCAL_CPU_SIZE=1.0</blockquote></details> <details> <summary>components/backends/vllm/deploy/README.md (1)</summary><blockquote> `205-208`: **Minor typos in example prompt text.** Non-blocking, but consider fixing to avoid distracting readers. ```diff - ... hinting at ests that Aeloria holds a secret ... + ... hinting at suggests that Aeloria holds a secret ... - ... a search for lost familt clue is hidden. + ... a search for lost family; the clue is hidden.docs/components/router/README.md (1)
22-22: Minor phrasing nit.Consider clarifying “configurable via --http-port” to be explicit.
-- Exposes the service on port 8000 (configurable) +- Exposes the service on port 8000 (configurable via --http-port)docs/guides/planner_benchmark/README.md (1)
79-79: Second genai-perf example also on :8000 — good.Optional consistency: either include
--service-kind openaiin both examples or neither.-genai-perf profile \ +genai-perf profile \ --tokenizer deepseek-ai/DeepSeek-R1-Distill-Llama-8B \ -m deepseek-ai/DeepSeek-R1-Distill-Llama-8B \ - --endpoint-type chat \ + --service-kind openai \ + --endpoint-type chat \ --url http://localhost:8000 \ --streaming \README.md (1)
133-133: Nit: add scheme for clarity and curl robustness.Explicit scheme avoids proxy/env surprises.
-curl localhost:8000/v1/chat/completions -H "Content-Type: application/json" -d '{ +curl http://localhost:8000/v1/chat/completions -H "Content-Type: application/json" -d '{Optional: add an HTTPS example above when TLS flags are used:
curl https://localhost:8000/v1/chat/completions -k -H "Content-Type: application/json" -d '{ ... }'
📜 Review details
Configuration used: Path: .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 (16)
README.md(2 hunks)components/backends/mocker/README.md(1 hunks)components/backends/vllm/deepseek-r1.md(1 hunks)components/backends/vllm/deploy/README.md(1 hunks)components/frontend/README.md(1 hunks)deploy/metrics/README.md(1 hunks)docs/architecture/dynamo_flow.md(2 hunks)docs/components/router/README.md(1 hunks)docs/guides/dynamo_deploy/create_deployment.md(1 hunks)docs/guides/dynamo_run.md(1 hunks)docs/guides/metrics.md(1 hunks)docs/guides/planner_benchmark/README.md(2 hunks)docs/support_matrix.md(1 hunks)examples/multimodal/README.md(6 hunks)lib/runtime/examples/system_metrics/README.md(2 hunks)tests/lmcache/README.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: PeaBrane
PR: ai-dynamo/dynamo#2756
File: lib/llm/src/kv_router/subscriber.rs:36-44
Timestamp: 2025-08-29T10:03:48.330Z
Learning: PeaBrane prefers to keep PRs contained in scope and is willing to defer technical improvements to future PRs when the current implementation works for the immediate use case. They acknowledge technical debt but prioritize deliverability over completeness in individual PRs.
🪛 markdownlint-cli2 (0.17.2)
components/backends/mocker/README.md
40-40: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
40-40: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
tests/lmcache/README.md
70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/components/router/README.md
[grammar] ~20-~20: There might be a mistake here.
Context: ...e kv --http-port 8000 ``` This command: - Launches the Dynamo frontend service wit...
(QB_NEW_EN)
⏰ 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). (1)
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (15)
docs/guides/dynamo_deploy/create_deployment.md (1)
91-93: Verify default HTTP port in dynamo.frontend CLI (docs/guides/dynamo_deploy/create_deployment.md:91–93)
Cannot locate the--http-portdefault in thedynamo.frontendimplementation; please confirm it remains set to8000to keep the docs accurate.lib/runtime/examples/system_metrics/README.md (2)
188-193: LGTM: frontend default port note matches the PR change.
205-206: LGTM: metrics example updated to port 8000.components/backends/mocker/README.md (1)
39-41: Port change to 8000 looks correct.The frontend start command now matches the new default port.
examples/multimodal/README.md (1)
72-101: Port updates to 8000 across all client examples look good.All curl samples consistently target localhost:8000.
Also applies to: 148-174, 224-251, 296-323, 368-393, 456-482
components/frontend/README.md (1)
3-3: Usage line updated to port 8000 — looks good.components/backends/vllm/deploy/README.md (2)
165-168: Port-forward example updated to 8000 — looks good.
199-213: Test request now targets port 8000 — looks good.docs/architecture/dynamo_flow.md (2)
26-26: Port updated to 8000 — consistent with PR intent.Reads clearly and matches the new default.
87-87: Diagram label updated to 8000 — looks good.Mermaid node text aligns with the prose change.
deploy/metrics/README.md (2)
26-26: Metrics scrape target moved to :8000 — correct.Topology now reflects FE on 8000 with /metrics.
119-127: Ensure prometheus.yml matches the new FE port.If prometheus.yml still targets :8080/metrics, scrapes will fail.
Use the repo-wide scan provided in docs/architecture/dynamo_flow.md comment to confirm prometheus.yml is aligned.
docs/components/router/README.md (1)
17-18: Quick start command switched to 8000 — OK.Command is copy-pastable and consistent.
docs/guides/planner_benchmark/README.md (1)
49-49: URL updated to localhost:8000 — good.docs/guides/metrics.md (1)
82-82: Diagram scrape edge switched to :8000/metrics — aligned with deploy docs.
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
indrajit96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com> Signed-off-by: nnshah1 <neelays@nvidia.com>
Overview:
as titled, a follow up for #2853
Summary by CodeRabbit