Skip to content

Conversation

keivenchang
Copy link
Contributor

@keivenchang keivenchang commented Oct 4, 2025

Overview:

Rename deploy/dynamo_check.py to deploy/sanity_check.py for better clarity and add documentation in README about running the sanity check before trying out Dynamo.

Details:

  • Rename deploy/dynamo_check.py to deploy/sanity_check.py with backward compatibility symlink
  • Add "Sanity check (optional)" section in README.md before "Running an LLM API server" section
  • Update .devcontainer/post-create.sh reference to use new path

Where should the reviewer start?

README.md for the new documentation section, then deploy/sanity_check.py for the updated usage instructions.

/coderabbit profile chill

Summary by CodeRabbit

  • New Features
    • Added a comprehensive sanity check command to diagnose system readiness (GPU, dependencies, frameworks, workspace). Supports optional thorough mode and terse output, renders a clear status tree, and returns pass/fail exit codes for local and CI use.
  • Documentation
    • Updated README with an optional “Sanity check” section and usage instructions.
  • Chores
    • Updated development container post-create flow to run the new sanity check script.

- Rename deploy/dynamo_check.py to deploy/sanity_check.py
- Update README.md to mention sanity check before trying out Dynamo
- Update usage instructions in sanity_check.py to reflect new path

Signed-off-by: Keiven Chang <[email protected]>
@keivenchang keivenchang requested a review from a team as a code owner October 4, 2025 01:37
@keivenchang keivenchang self-assigned this Oct 4, 2025
Copy link
Contributor

coderabbitai bot commented Oct 4, 2025

Walkthrough

Replaced post-create sanity check invocation to use deploy/sanity_check.py, updated README with an optional sanity check section, introduced a new deploy/sanity_check.py diagnostic tool with CLI and hierarchical checks, and touched deploy/dynamo_check.py metadata without exposing functional changes.

Changes

Cohort / File(s) Summary of Changes
Devcontainer post-create flow
\.devcontainer/post-create.sh
Switched verification step from deploy/dynamo_check.py to deploy/sanity_check.py after build.
Documentation update
README.md
Added “Sanity check (optional)” section with command to run ./deploy/sanity_check.py. No other flow changes.
System diagnostics tool
deploy/sanity_check.py
New comprehensive system/workspace checker with tree-based reporting, GPU/OS/Python/Rust/Cargo/framework introspection, workspace detection, CLI flags (--thorough-check, --terse), and exit codes based on detected errors.
Legacy checker reference
deploy/dynamo_check.py
Noted alongside creation of the new sanity_check.py; no functional API surfaced in diff.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Devcontainer
  participant SH as post-create.sh
  participant SC as sanity_check.py

  Dev->>SH: Execute post-create
  SH->>SC: Run sanity_check.py (default mode)
  SC-->>SH: Print tree + exit code (0/1)
  alt Exit code 0
    SH-->>Dev: Continue post-create flow
  else Exit code 1
    SH-->>Dev: Report failure (logs preserved)
  end
Loading
sequenceDiagram
  autonumber
  actor User
  participant CLI as Shell
  participant SC as sanity_check.py
  participant Sys as System/Env
  participant Dyn as Dynamo Workspace
  participant FW as Frameworks (vLLM/Sglang/TensorRT-LLM)

  User->>CLI: ./deploy/sanity_check.py [--thorough-check] [--terse]
  CLI->>SC: Invoke main()
  SC->>Sys: Collect OS/Python/GPU/Rust/Cargo
  SC->>FW: Detect frameworks and versions
  SC->>Dyn: Locate workspace and runtime/framework installs
  SC-->>CLI: Render tree + exit code (0/1)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

A rabbit tapped the shell with cheer,
“Sanity check, the path is clear!”
GPUs hum, Rust tools align,
Python whispers, versions fine.
If frameworks blink with little glow—
I’ll map the tree so you will know.
Hop hop: green lights, on we go! 🐇✨

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description follows the required template with clear Overview, Details, and reviewer guidance sections, but it omits the mandatory "Related Issues" section specified by the template. Please add a "#### Related Issues" section to link relevant GitHub issues using the appropriate action keywords (Closes, Fixes, Resolves, etc.).
Docstring Coverage ⚠️ Warning Docstring coverage is 74.55% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately summarizes the primary change by indicating that the script file is being renamed from dynamo_check.py to sanity_check.py, which aligns with the commit content and makes the purpose clear to reviewers.

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 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 30610e7 and 6cd7750.

📒 Files selected for processing (5)
  • .devcontainer/post-create.sh (1 hunks)
  • README.md (1 hunks)
  • deploy/dynamo_check.py (0 hunks)
  • deploy/dynamo_check.py (1 hunks)
  • deploy/sanity_check.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
deploy/sanity_check.py (1)
deploy/dynamo_check.py (45)
  • Colors (101-105)
  • NodeStatus (108-116)
  • NodeInfo (120-276)
  • add_child (137-140)
  • add_metadata (142-145)
  • render (147-212)
  • print_tree (214-217)
  • has_errors (219-230)
  • _replace_home_with_var (232-237)
  • _is_inside_container (239-253)
  • _check_cgroup_for_container (255-265)
  • _get_gpu_container_remedies (267-269)
  • _format_timestamp_pdt (271-276)
  • SystemInfo (279-385)
  • _get_ip_address (339-360)
  • OSInfo (423-480)
  • UserInfo (388-420)
  • GPUInfo (483-700)
  • FrameworkInfo (1581-1635)
  • FilePermissionsInfo (703-1224)
  • CargoInfo (1227-1478)
  • MaturinInfo (1481-1514)
  • PythonInfo (1517-1578)
  • _add_error_only_components (372-385)
  • DynamoInfo (2030-2175)
  • _check_permissions_unified (734-927)
  • _is_effectively_writable (929-954)
  • _format_disk_space (1188-1224)
  • _count_writable_files (956-1002)
  • _create_file_count_description (1004-1016)
  • _get_cargo_target_path_candidates (1018-1047)
  • find_workspace (2130-2151)
  • is_dynamo_workspace (2154-2175)
  • format_bytes (1204-1210)
  • _get_directory_size_gb (1330-1345)
  • DynamoRuntimeInfo (1714-1896)
  • _find_dist_info (1838-1863)
  • _find_pth_file (1865-1896)
  • _discover_runtime_components (1809-1836)
  • DynamoFrameworkInfo (1899-2027)
  • _discover_framework_components (2002-2027)
  • _get_git_info (2085-2127)
  • has_framework_errors (2178-2188)
  • show_installation_recommendation (2191-2196)
  • main (2199-2236)
🪛 Gitleaks (8.28.0)
deploy/sanity_check.py

[high] 50-50: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 Ruff (0.13.3)
deploy/sanity_check.py

264-264: Do not catch blind exception: Exception

(BLE001)


358-358: Consider moving this statement to an else block

(TRY300)


359-359: Do not catch blind exception: Exception

(BLE001)


382-382: Loop control variable name not used within loop body

Rename unused name to _name

(B007)


399-399: Do not catch blind exception: Exception

(BLE001)


404-404: Starting a process with a partial executable path

(S607)


408-409: try-except-pass detected, consider logging the exception

(S110)


408-408: Do not catch blind exception: Exception

(BLE001)


441-442: try-except-pass detected, consider logging the exception

(S110)


441-441: Do not catch blind exception: Exception

(BLE001)


460-461: try-except-pass detected, consider logging the exception

(S110)


460-460: Do not catch blind exception: Exception

(BLE001)


508-508: subprocess call: check for execution of untrusted input

(S603)


607-607: Do not catch blind exception: Exception

(BLE001)


619-619: subprocess call: check for execution of untrusted input

(S603)


629-629: subprocess call: check for execution of untrusted input

(S603)


638-639: try-except-pass detected, consider logging the exception

(S110)


638-638: Do not catch blind exception: Exception

(BLE001)


660-660: subprocess call: check for execution of untrusted input

(S603)


690-691: try-except-pass detected, consider logging the exception

(S110)


690-690: Do not catch blind exception: Exception

(BLE001)


698-699: try-except-pass detected, consider logging the exception

(S110)


698-698: Do not catch blind exception: Exception

(BLE001)


818-818: Do not catch blind exception: Exception

(BLE001)


821-821: Do not catch blind exception: Exception

(BLE001)


872-872: Do not catch blind exception: Exception

(BLE001)


880-880: Do not catch blind exception: Exception

(BLE001)


918-918: Do not catch blind exception: Exception

(BLE001)


922-922: Use explicit conversion flag

Replace with conversion flag

(RUF010)


951-951: Consider moving this statement to an else block

(TRY300)


952-952: Do not catch blind exception: Exception

(BLE001)


974-974: Loop control variable dirs not used within loop body

Rename unused dirs to _dirs

(B007)


998-998: Do not catch blind exception: Exception

(BLE001)


1025-1025: Starting a process with a partial executable path

(S607)


1038-1039: try-except-pass detected, consider logging the exception

(S110)


1038-1038: Do not catch blind exception: Exception

(BLE001)


1123-1123: Do not catch blind exception: Exception

(BLE001)


1127-1127: Use explicit conversion flag

Replace with conversion flag

(RUF010)


1221-1221: Consider moving this statement to an else block

(TRY300)


1223-1223: Do not catch blind exception: Exception

(BLE001)


1239-1239: Starting a process with a partial executable path

(S607)


1243-1244: try-except-pass detected, consider logging the exception

(S110)


1243-1243: Do not catch blind exception: Exception

(BLE001)


1334-1334: subprocess call: check for execution of untrusted input

(S603)


1335-1335: Starting a process with a partial executable path

(S607)


1343-1344: try-except-pass detected, consider logging the exception

(S110)


1343-1343: Do not catch blind exception: Exception

(BLE001)


1365-1365: subprocess call: check for execution of untrusted input

(S603)


1371-1372: try-except-pass detected, consider logging the exception

(S110)


1371-1371: Do not catch blind exception: Exception

(BLE001)


1395-1395: Do not catch blind exception: Exception

(BLE001)


1418-1418: Do not catch blind exception: Exception

(BLE001)


1438-1439: try-except-pass detected, consider logging the exception

(S110)


1438-1438: Do not catch blind exception: Exception

(BLE001)


1445-1445: Do not catch blind exception: Exception

(BLE001)


1475-1476: try-except-pass detected, consider logging the exception

(S110)


1475-1475: Do not catch blind exception: Exception

(BLE001)


1499-1499: Starting a process with a partial executable path

(S607)


1511-1512: try-except-pass detected, consider logging the exception

(S110)


1511-1511: Do not catch blind exception: Exception

(BLE001)


1546-1547: try-except-pass detected, consider logging the exception

(S110)


1546-1546: Do not catch blind exception: Exception

(BLE001)


1623-1625: try-except-pass detected, consider logging the exception

(S110)


1623-1623: Do not catch blind exception: Exception

(BLE001)


1632-1632: Loop control variable display_name not used within loop body

(B007)


1726-1726: Do not catch blind exception: Exception

(BLE001)


1771-1772: try-except-pass detected, consider logging the exception

(S110)


1771-1771: Do not catch blind exception: Exception

(BLE001)


1857-1857: Do not catch blind exception: Exception

(BLE001)


1893-1893: Consider moving this statement to an else block

(TRY300)


1894-1894: Do not catch blind exception: Exception

(BLE001)


1911-1911: Do not catch blind exception: Exception

(BLE001)


1943-1943: Do not catch blind exception: Exception

(BLE001)


2090-2090: Starting a process with a partial executable path

(S607)


2100-2100: Starting a process with a partial executable path

(S607)


2120-2120: Do not catch blind exception: Exception

(BLE001)


2125-2125: Consider moving this statement to an else block

(TRY300)


2126-2126: Do not catch blind exception: Exception

(BLE001)

⏰ 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). (6)
  • GitHub Check: vllm (arm64)
  • GitHub Check: trtllm (arm64)
  • GitHub Check: trtllm (amd64)
  • GitHub Check: vllm (amd64)
  • GitHub Check: sglang
  • GitHub Check: Build and Test - dynamo

@hhzhang16
Copy link
Contributor

Why did you choose to symlink dynamo_check.py to sanity_check.py instead of purely renaming the file?

@keivenchang
Copy link
Contributor Author

Why did you choose to symlink dynamo_check.py to sanity_check.py instead of purely renaming the file?

Ah you caught that :) There might be people who are still calling dynamo_check.py, so for backward compatibility I symlinked it to sanity_check.py, with the intention to remove the symlink completely after a release or two.

@keivenchang keivenchang merged commit 1ddb9b0 into main Oct 7, 2025
19 of 21 checks passed
@keivenchang keivenchang deleted the keivenchang/sanity_check.py branch October 7, 2025 20:05
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