-
Notifications
You must be signed in to change notification settings - Fork 692
fix: Update quickstart.md #2414
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
updated this to reflect 0.4.0 and API key dependency Signed-off-by: Anish <80174047+athreesh@users.noreply.github.com>
WalkthroughUpdates the dynamo_deploy quickstart documentation: increases RELEASE_VERSION from 0.3.2 to 0.4.0 and removes the “Authenticate with NGC” section. Remaining steps, including fetching Helm charts, are unchanged. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 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: 1
🔭 Outside diff range comments (3)
docs/guides/dynamo_deploy/quickstart.md (3)
88-90: Fix push tag: pushing latest-vllm while tagging $IMAGE_TAG.You tag the image with $IMAGE_TAG but push latest-vllm. This will fail or push the wrong tag.
Apply:
-docker push <your-registry>/dynamo-base:latest-vllm +docker push <your-registry>/dynamo-base:$IMAGE_TAG
24-32: Chart URLs publicly accessible; switch tohelm pull
Bothdynamo-crds-0.4.0.tgzanddynamo-platform-0.4.0.tgzreturn HTTP 200 without credentials.Please update the guide to use Helm v3’s modern command:
--- docs/guides/dynamo_deploy/quickstart.md @@ Fetch Helm Charts -helm fetch https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-crds-${RELEASE_VERSION}.tgz +helm pull https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-crds-${RELEASE_VERSION}.tgz -helm fetch https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-platform-${RELEASE_VERSION}.tgz +helm pull https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-platform-${RELEASE_VERSION}.tgz
79-80: Update hard-coded examples to v0.4.0
- docs/guides/dynamo_deploy/quickstart.md (line 79):
-export IMAGE_TAG=RELEASE_VERSION # i.e. 0.3.2 - the release you are using +export IMAGE_TAG=RELEASE_VERSION # i.e. 0.4.0 - the release you are using- examples/deployments/AKS/AKS-deployment.md (line 96):
-export RELEASE_VERSION=0.3.2 +export RELEASE_VERSION=0.4.0- examples/deployments/EKS/Deploy_Dynamo_Cloud.md (line 19):
-export IMAGE_TAG=0.3.2.1 +export IMAGE_TAG=0.4.0.1Consider sweeping other 0.3.x occurrences in documentation (e.g. components/backends/sglang/README.md:107) to keep examples in sync.
🧹 Nitpick comments (3)
docs/guides/dynamo_deploy/quickstart.md (3)
101-101: Nit: double space and superfluous “the”.Minor grammar/readability tweak.
Apply:
-The Nvidia Cloud Operator image will be pulled from the `$DOCKER_SERVER/dynamo-operator:$IMAGE_TAG`. +The Nvidia Cloud Operator image will be pulled from `$DOCKER_SERVER/dynamo-operator:$IMAGE_TAG`.
107-116: Clarify that image pull secret is optional for public nvcr images.If 0.4.0 images are public (as implied by removing API key dependency), readers using nvcr.io shouldn’t need a registry secret.
Apply:
-Create the namespace and the docker registry secret. +Create the namespace and, if your image registry requires authentication, the docker registry secret. +Note: If you are using the public nvcr.io images for release 0.4.0, you can skip creating the image pull secret.
136-146: Fix mismatched emphasis markers in headings.Leading/trailing asterisks are inconsistent; renders oddly.
Apply:
-***Step 1: Install Custom Resource Definitions (CRDs)** +**Step 1: Install Custom Resource Definitions (CRDs)** -***Step 2: Build Dependencies and Install Platform** +**Step 2: Build Dependencies and Install Platform**
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/guides/dynamo_deploy/quickstart.md(1 hunks)
⏰ 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 (1)
docs/guides/dynamo_deploy/quickstart.md (1)
15-18: RELEASE_VERSION bump to 0.4.0 looks good.This aligns with the PR objective and the high-level summary.
|
|
||
| ```bash | ||
| helm repo add nvidia https://helm.ngc.nvidia.com/nvidia --username='$oauthtoken' --password=<YOUR_NGC_CLI_API_KEY> | ||
| ``` |
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.
Stray markdown code fence breaks rendering.
There’s a lone triple backtick with no matching pair, which will corrupt subsequent markdown blocks.
Apply this diff to remove it:
-```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/guides/dynamo_deploy/quickstart.md at line 22 there is a stray triple
backtick that opens a code fence with no matching closing fence; remove that
lone ``` so the markdown rendering is not broken and subsequent blocks render
correctly.
super small fix- updated this guide to reflect 0.4.0 and remove API key dependency
Overview:
Details:
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit