Skip to content

Fix: use python3 in debugger server.sh#1590

Merged
cjluo-nv merged 1 commit into
mainfrom
chenjiel/debugger-python3
Jun 1, 2026
Merged

Fix: use python3 in debugger server.sh#1590
cjluo-nv merged 1 commit into
mainfrom
chenjiel/debugger-python3

Conversation

@cjluo-nv
Copy link
Copy Markdown
Collaborator

@cjluo-nv cjluo-nv commented Jun 1, 2026

What does this PR do?

Type of change: Bug fix

tools/debugger/server.sh invokes python -c "..." inside check_modelopt_local (and pip install -e .[dev] in the fallback path). Containers that only ship python3 (no python shim) cause the check to fail with python: command not found, exit 127. The server interprets this as "modelopt not editable-installed", runs pip install, the second check fails the same way, and the server aborts before it ever listens for commands.

Switches both the inline import check and the install fallback to python3 / python3 -m pip.

Usage

bash tools/debugger/server.sh
# now starts cleanly on python3-only images

Testing

Verified inside a container where which python returns nothing and python3 is /usr/bin/python3 (Python 3.12). With the old script, server.sh aborted at check_modelopt_local. With this fix, the check passes against an existing editable install and the server proceeds to wait for the client handshake.

Before your PR is "Ready for review"

  • Is this change backward compatible?: ✅ — python3 is present on every image that previously had python.
  • If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
  • Did you write any new necessary tests?: N/A — internal tooling shell script.
  • Did you update Changelog?: N/A — internal debugger tooling, not user-facing.
  • Did you get Claude approval on this PR?: N/A

Summary by CodeRabbit

  • Chores
    • Updated debugger tooling to ensure consistent use of Python 3 for package validation and installation processes.

Containers without a `python` shim (only `python3`) caused
`check_modelopt_local` in `tools/debugger/server.sh` to fail with
`python: command not found`, aborting the server before it could
verify the editable install. Switch the inline check and the
fallback `pip install -e .[dev]` invocation to `python3` /
`python3 -m pip` so the script works on python3-only images.

Signed-off-by: Chenjie Luo <chenjiel@nvidia.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 69c2aeeb-281c-48fa-88b4-892006938f5f

📥 Commits

Reviewing files that changed from the base of the PR and between 38c7843 and 7d6e7c1.

📒 Files selected for processing (1)
  • tools/debugger/server.sh

📝 Walkthrough

Walkthrough

The debugger server shell script is updated to consistently use python3 instead of python when validating and installing the local modelopt package. The validation check and installation command now both explicitly invoke python3, with error messaging updated accordingly.

Changes

Python 3 consistency in debugger server script

Layer / File(s) Summary
Python 3 command consistency
tools/debugger/server.sh
The check_modelopt_local validation now runs python3 -c instead of python -c, and the installation path runs python3 -m pip install -e ".[dev]" with matching error guidance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: updating debugger server.sh to use python3 instead of python for consistency.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns ✅ Passed PR modifies only bash script tools/debugger/server.sh. Check applies to modelopt Python changes; no Python security anti-patterns found.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chenjiel/debugger-python3

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

Copy link
Copy Markdown
Contributor

@meenchen meenchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot review — DM the bot to share feedback.

Trivial, correct fix: switches pythonpython3 and pippython3 -m pip in tools/debugger/server.sh so the editable-install check works in containers that only ship python3. Internal debugger tooling, no tests warranted, fully backward-compatible.

@cjluo-nv cjluo-nv enabled auto-merge (squash) June 1, 2026 21:40
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.88%. Comparing base (54fb87e) to head (7d6e7c1).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1590      +/-   ##
==========================================
- Coverage   73.22%   72.88%   -0.35%     
==========================================
  Files         478      479       +1     
  Lines       52421    52735     +314     
==========================================
+ Hits        38387    38436      +49     
- Misses      14034    14299     +265     
Flag Coverage Δ
unit 53.62% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cjluo-nv cjluo-nv merged commit 905259f into main Jun 1, 2026
34 checks passed
@cjluo-nv cjluo-nv deleted the chenjiel/debugger-python3 branch June 1, 2026 21:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-01 21:47 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants