Skip to content

fix: typo from mountPoint to mountPath - #4252

Merged
dagil-nvidia merged 1 commit into
mainfrom
bug-5655674-fix
Nov 12, 2025
Merged

fix: typo from mountPoint to mountPath#4252
dagil-nvidia merged 1 commit into
mainfrom
bug-5655674-fix

Conversation

@hhzhang16

@hhzhang16 hhzhang16 commented Nov 12, 2025

Copy link
Copy Markdown
Contributor

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

Summary by CodeRabbit

  • Bug Fixes
    • Corrected volume mount configuration naming in Planner settings to align with standard conventions.

Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
@hhzhang16
hhzhang16 requested a review from a team as a code owner November 12, 2025 11:58
@hhzhang16
hhzhang16 requested a review from a team November 12, 2025 11:58
@github-actions github-actions Bot added the fix label Nov 12, 2025
@coderabbitai

coderabbitai Bot commented Nov 12, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

A field name correction in the Planner ConfigMap volume mount configuration, renaming "mountPoint" to "mountPath" in the profiler utilities for the planner-profile-data volume.

Changes

Cohort / File(s) Summary
Volume mount field rename
benchmarks/profiler/utils/dgd_generation.py
Renamed "mountPoint" field to "mountPath" in Planner extraPodSpec mainContainer volume mount configuration

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A mount point becomes a path so true,
One little rename, a field made new,
In profiler's realm where volumes dance,
The planner springs forward in steady prance,
Small tweaks of precision, our code refined! 🎯

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only placeholder template text with no actual content filled in, making it impossible to understand the context, motivation, or scope of the changes. Fill in the Overview and Details sections with a clear explanation of the typo fix, specify which files to review, and replace the placeholder issue number with the actual GitHub issue being closed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing a typo by renaming 'mountPoint' to 'mountPath', which directly matches the file-level summary.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd96a2 and b89d03f.

📒 Files selected for processing (1)
  • benchmarks/profiler/utils/dgd_generation.py (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nv-hwoo
Repo: ai-dynamo/dynamo PR: 4112
File: examples/backends/sglang/deploy/agg.yaml:76-78
Timestamp: 2025-11-05T20:23:29.539Z
Learning: In DynamoGraphDeployment (DGD) YAML manifests, the volumeMounts field uses `mountPoint` (not the standard Kubernetes `mountPath`) to specify where to mount volumes. This is defined in the DGD custom resource definition API.
📚 Learning: 2025-11-05T20:23:29.539Z
Learnt from: nv-hwoo
Repo: ai-dynamo/dynamo PR: 4112
File: examples/backends/sglang/deploy/agg.yaml:76-78
Timestamp: 2025-11-05T20:23:29.539Z
Learning: In DynamoGraphDeployment (DGD) YAML manifests, the volumeMounts field uses `mountPoint` (not the standard Kubernetes `mountPath`) to specify where to mount volumes. This is defined in the DGD custom resource definition API.

Applied to files:

  • benchmarks/profiler/utils/dgd_generation.py
⏰ 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)
benchmarks/profiler/utils/dgd_generation.py (1)

206-212: Potential regression: changing mountPoint to mountPath may break DGD manifests.

According to retrieved learnings from PR #4112, DynamoGraphDeployment (DGD) YAML manifests use mountPoint (not the standard Kubernetes mountPath) to specify volume mount locations in the custom resource definition. The change on line 209 converts this to mountPath, which would be incompatible with the DGD CRD API.

This appears to be a regression rather than a fix. The PR title suggests correcting a typo, but the DGD specification requires mountPoint.

Please verify:

  1. Is mountPath intentionally replacing mountPoint for DGD volumeMounts?
  2. Does this align with the DGD custom resource definition as of the current version?
  3. If the change is correct, has the DGD CRD API been updated to use mountPath?

Based on learnings from previous review (nv-hwoo, PR #4112).


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dagil-nvidia
dagil-nvidia merged commit a207b4b into main Nov 12, 2025
24 of 38 checks passed
@dagil-nvidia
dagil-nvidia deleted the bug-5655674-fix branch November 12, 2025 16:32
dagil-nvidia pushed a commit that referenced this pull request Nov 12, 2025
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
nv-anants pushed a commit that referenced this pull request Nov 12, 2025
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
Co-authored-by: hhzhang16 <54051230+hhzhang16@users.noreply.github.com>
daiyaanarfeen pushed a commit that referenced this pull request Nov 14, 2025
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Signed-off-by: Daiyaan <darfeen@nvidia.com>
tangcy98 pushed a commit to tangcy98/dynamo that referenced this pull request Nov 14, 2025
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
yao531441 pushed a commit to yao531441/dynamo that referenced this pull request May 13, 2026
Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants