Skip to content

[None][build] Add --yes flag to build_wheel.py to skip interactive prompt#16015

Merged
brnguyen2 merged 1 commit into
NVIDIA:mainfrom
brnguyen2:brnguyen/build-wheel-yes-flag
Jul 7, 2026
Merged

[None][build] Add --yes flag to build_wheel.py to skip interactive prompt#16015
brnguyen2 merged 1 commit into
NVIDIA:mainfrom
brnguyen2:brnguyen/build-wheel-yes-flag

Conversation

@brnguyen2

@brnguyen2 brnguyen2 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • build_wheel.py blocks indefinitely on an interactive input() prompt when it detects PyPI PyTorch inside an NVIDIA container venv. This blocks non-interactive builds that run inside a pseudo-TTY (e.g. via script(1)), since isatty() returns True in that case.
  • Add --yes / -y flag to build_wheel.py (and thread it through setup_venv) to skip the prompt while still printing the warning.

Test plan

  • Run build_wheel.py --yes --install ... in a container with an existing .venv that has PyPI PyTorch — confirm it prints the warning but does not block.
  • Run without --yes in an interactive shell — confirm the prompt still appears.

Summary by CodeRabbit

  • New Features
    • Added a --yes / -y option to run builds without interactive confirmation prompts.
    • Non-interactive builds now continue automatically when a compatibility warning appears.

…ompt

When running inside a pseudo-TTY (e.g. via script(1) or CI), the
PyTorch compatibility warning in build_wheel.py blocks indefinitely
waiting for a keypress. Add --yes / -y to suppress the prompt while
still printing the warning text.

Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a --yes/-y CLI flag to scripts/build_wheel.py that skips interactive confirmation prompts. setup_venv and main() gain a yes: bool = False parameter, and the NVIDIA PyTorch container compatibility warning prompt now only triggers when yes is not set.

Changes

Non-interactive Build Flag

Layer / File(s) Summary
Prompt gating in setup_venv
scripts/build_wheel.py
setup_venv accepts a new yes parameter; the interactive input() prompt for NVIDIA PyTorch container compatibility warnings is now conditional on yes being false.
CLI wiring for --yes flag
scripts/build_wheel.py
main() adds a yes: bool = False keyword parameter passed to setup_venv, and add_arguments() registers the --yes/-y option with help text.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers summary and tests, but it omits the required Description and PR Checklist sections from the template. Add a short Description section and complete the PR Checklist, or clearly note any intentionally omitted items.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title is specific and matches the main change: adding a yes flag to skip the interactive prompt.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
scripts/build_wheel.py (1)

138-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Docstring not updated for new yes parameter.

The yes parameter is added to the signature but the docstring's Args section still only documents project_dir, requirements_file, and no_venv (and even no_venv was already undocumented). As per coding guidelines, "Externally called functions should have docstrings, and their arguments should be documented."

📝 Suggested docstring update
     Args:
         project_dir: The root directory of the project.
         requirements_file: Path to the requirements file.
+        no_venv: Use current Python environment as is.
+        yes: Skip interactive confirmation prompts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/build_wheel.py` around lines 138 - 176, Update the setup_venv
docstring to document all callable arguments, including the new yes parameter
and the existing no_venv flag, so the Args section matches the function
signature. Locate setup_venv in build_wheel.py and add a brief description for
each parameter, keeping the wording consistent with the rest of the file’s
docstrings.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@scripts/build_wheel.py`:
- Around line 138-176: Update the setup_venv docstring to document all callable
arguments, including the new yes parameter and the existing no_venv flag, so the
Args section matches the function signature. Locate setup_venv in build_wheel.py
and add a brief description for each parameter, keeping the wording consistent
with the rest of the file’s docstrings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 188e9989-40c5-4c59-ae7b-bd2988f7409b

📥 Commits

Reviewing files that changed from the base of the PR and between a0c406f and d654418.

📒 Files selected for processing (1)
  • scripts/build_wheel.py

@brnguyen2

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #57877 [ run ] triggered by Bot. Commit: d654418 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #57877 [ run ] completed with state FAILURE. Commit: d654418
/LLM/main/L0_MergeRequest_PR pipeline #46573 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@brnguyen2 brnguyen2 requested a review from tburt-nv July 7, 2026 17:53
@tburt-nv

tburt-nv commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

/bot skip --comment "build stages passed"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #58067 [ skip ] triggered by Bot. Commit: d654418 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #58067 [ skip ] completed with state SUCCESS. Commit: d654418
Skipping testing for commit d654418

Link to invocation

@brnguyen2 brnguyen2 merged commit 65f1612 into NVIDIA:main Jul 7, 2026
14 of 17 checks passed
crazydemo pushed a commit to crazydemo/TensorRT-LLM that referenced this pull request Jul 8, 2026
…ompt (NVIDIA#16015)

Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
BrianLi23 pushed a commit to BrianLi23/TensorRT-LLM that referenced this pull request Jul 9, 2026
…ompt (NVIDIA#16015)

Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
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.

3 participants