Revert "Move pip install to dedicated GitHub Actions steps"#219
Conversation
This reverts commit b1df040.
functionstackx
deleted the
revert-206-copilot/update-github-action-workflow
branch
There was a problem hiding this comment.
Pull Request Overview
This PR reverts a previous change that moved pip install commands to dedicated GitHub Actions steps. The revert consolidates dependency installation back into the run commands where the dependencies are used.
Key Changes:
- Removes dedicated "Set up Python" and "Install dependencies" steps from workflow files
- Moves
pip installcommands inline with the Python scripts that use them - Adjusts the timing of dependency installation to occur immediately before usage
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/label-validation.yml | Removes Python setup steps and moves pip install inline with Python script execution |
| .github/workflows/full-sweep-test.yml | Removes Python setup steps and moves pip install inline with script execution |
| .github/workflows/full-sweep-8k1k-scheduler.yml | Removes Python setup steps and moves pip install inline with script execution |
| .github/workflows/full-sweep-1k8k-scheduler.yml | Removes Python setup steps and moves pip install inline with script execution |
| .github/workflows/full-sweep-1k1k-scheduler.yml | Removes Python setup steps and moves pip install inline with script execution |
| .github/workflows/e2e-tests.yml | Removes Python setup steps and moves pip install inline with script execution |
| .github/workflows/collect-results.yml | Removes Python setup step and moves matplotlib installation inline with plotting script |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Generate configs for standard labels | ||
| all_configs = [] | ||
| if matching: | ||
| subprocess.run(['pip', 'install', 'pydantic'], check=True) |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the script requires the pyyaml package.
| subprocess.run(['pip', 'install', 'pydantic'], check=True) | |
| subprocess.run(['pip', 'install', 'pydantic', 'pyyaml'], check=True) |
| # discrete inputs (i.e., run_1k1k, run_h100, etc.) to split the test sweep into discrete jobs | ||
| - id: generate-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
| pip install pydantic | |
| pip install pydantic pyyaml |
|
|
||
| - id: get-dsr1-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
|
|
||
| - id: get-gptoss-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
|
|
||
| - id: get-dsr1-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
|
|
||
| - id: get-gptoss-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
|
|
||
| - id: get-dsr1-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
|
|
||
| - id: get-gptoss-configs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
|
|
||
| - id: get-jobs | ||
| run: | | ||
| pip install pydantic |
There was a problem hiding this comment.
The pyyaml dependency is missing from the pip install command. The original code installed both pydantic and pyyaml, but after the revert only pydantic is being installed. This will cause failures if the generate_sweep_configs.py script requires the pyyaml package.
| pip install pydantic | |
| pip install pydantic pyyaml |
Reverts InferenceMAX/InferenceMAX#206