fix: Verify $python_exec exists before running#79
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds an early validation check in the pyperf_run script to verify that the user-specified Python interpreter exists and is executable. If the interpreter is unavailable, the script exits with an error code instead of silently succeeding, addressing issue ChangesPython interpreter validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This relates to RPOPC-764 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pyperf/pyperf_run (1)
403-405: ⚡ Quick winRemove redundant validation check.
This check is now unreachable. The new validation at lines 395-397 exits before this point if
python_execis invalid. Removing this redundant check improves code clarity and eliminates dead code.♻️ Proposed cleanup
pkg_list="--packages `echo $python_pkgs | sed "s/,/ /g"`" fi -if ! command -v $python_exec; then - exit_out "Error: Designated python executable, $python_exec, not present" $E_GENERAL -fi if [[ $to_no_pkg_install -eq 0 ]]; then🤖 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 `@pyperf/pyperf_run` around lines 403 - 405, Remove the unreachable redundant validation block that checks for the python executable presence (the "if ! command -v $python_exec; then exit_out ... $E_GENERAL fi" block); since earlier validation already exits on an invalid python_exec, delete this entire conditional and its call to exit_out to eliminate dead code and avoid duplicate checks, and verify no other code paths depend on this specific check or on side effects of calling exit_out here.
🤖 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 `@pyperf/pyperf_run`:
- Around line 403-405: Remove the unreachable redundant validation block that
checks for the python executable presence (the "if ! command -v $python_exec;
then exit_out ... $E_GENERAL fi" block); since earlier validation already exits
on an invalid python_exec, delete this entire conditional and its call to
exit_out to eliminate dead code and avoid duplicate checks, and verify no other
code paths depend on this specific check or on side effects of calling exit_out
here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 96499ab1-3aa4-48e7-9149-a7d95a887270
📒 Files selected for processing (1)
pyperf/pyperf_run
Description
Verifies that $python_exec is a valid PATH binary or absolute path before running.
Before/After Comparison
Before
No verification is done
After
Wrapper exits out if the executable isn't in the PATH or the absolute file isn't executable.
Clerical Stuff
Closes #57
Relates to JIRA: RPOPC-764