Skip to content

Move pip install to dedicated GitHub Actions steps#206

Merged
cquil11 merged 4 commits into
mainfrom
copilot/update-github-action-workflow
Nov 10, 2025
Merged

Move pip install to dedicated GitHub Actions steps#206
cquil11 merged 4 commits into
mainfrom
copilot/update-github-action-workflow

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 9, 2025

Inline pip install commands in workflow run scripts should be moved to proper GitHub Actions steps for better organization and maintainability.

Changes

  • Removed inline pip install: Eliminated subprocess.run(['pip', 'install', 'pydantic']) and bash script pip commands from 8 workflow files
  • Added Python setup steps: Each job now includes actions/setup-python@v6 with Python 3.12 before dependency installation
  • Standardized pattern: All workflows use dedicated "Install dependencies" steps with only the packages they need
  • Optimized dependencies: Each workflow installs only required packages (e.g., label-validation.yml installs only pydantic, not pyyaml)
  • Preserved original behavior: Maintained the -q flag for quiet matplotlib installation in collect-results.yml

Example

Before:

- id: get-jobs
  run: |
    pip install pydantic
    CONFIG_JSON=$(python3 generate_sweep_configs.py)

After:

- name: Set up Python
  uses: actions/setup-python@v6
  with:
    python-version: '3.12'

- name: Install dependencies
  run: pip install pydantic pyyaml

- id: get-jobs
  run: |
    CONFIG_JSON=$(python3 generate_sweep_configs.py)

Files Modified

  • .github/workflows/e2e-tests.yml
  • .github/workflows/full-sweep-{1k1k,1k8k,8k1k}-scheduler.yml
  • .github/workflows/full-sweep-test.yml
  • .github/workflows/collect-results.yml
  • .github/workflows/label-validation.yml
  • .github/workflows/test-matrix-logic.yml

View original Slack conversation

View original Slack conversation


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Update GitHub Action to install pydantic using caching Move pip install to dedicated GitHub Actions steps with caching Nov 9, 2025
Copilot AI requested a review from functionstackx November 9, 2025 22:25
Co-authored-by: functionstackx <47992694+functionstackx@users.noreply.github.com>
@functionstackx functionstackx force-pushed the copilot/update-github-action-workflow branch from 0f30308 to 6fd5041 Compare November 9, 2025 22:48
@functionstackx functionstackx marked this pull request as ready for review November 9, 2025 22:48
@functionstackx functionstackx requested a review from a team as a code owner November 9, 2025 22:48
Copilot AI review requested due to automatic review settings November 9, 2025 22:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves GitHub Actions workflow efficiency by moving inline pip install commands into dedicated dependency installation steps with caching enabled. This allows GitHub Actions to cache Python packages between runs, reducing build times and network usage.

Key changes:

  • Added actions/setup-python@v6 with cache: 'pip' configuration to 8 workflow files
  • Removed inline pip install commands from bash scripts and Python subprocess calls
  • Created requirements-workflows.txt to centralize workflow dependencies

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
requirements-workflows.txt New file documenting all Python dependencies used across workflows
.github/workflows/test-matrix-logic.yml Added pip caching to existing Python setup step
.github/workflows/label-validation.yml Added Python setup with caching and removed inline pip installs from two jobs
.github/workflows/full-sweep-test.yml Added Python setup with caching and removed inline pip installs from config generation and results collection jobs
.github/workflows/full-sweep-8k1k-scheduler.yml Added Python setup with caching to three jobs (dsr1, gptoss config generation, and results collection)
.github/workflows/full-sweep-1k8k-scheduler.yml Added Python setup with caching to three jobs (dsr1, gptoss config generation, and results collection)
.github/workflows/full-sweep-1k1k-scheduler.yml Added Python setup with caching to three jobs (dsr1, gptoss config generation, and results collection)
.github/workflows/e2e-tests.yml Added Python setup with caching and removed inline pip installs from job generation and results collection
.github/workflows/collect-results.yml Added Python setup with caching and simplified plot performance step

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread requirements-workflows.txt Outdated
Comment on lines +1 to +7
# Requirements for GitHub Actions workflows
# Used by various workflow files to generate configs and process results
pydantic>=2.0.0
pyyaml>=6.0.0
PyGithub>=2.0.0
matplotlib>=3.0.0
pytest>=7.0.0
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The requirements-workflows.txt file is not referenced by any of the workflow files. The workflows install dependencies directly via pip install <package> commands instead of using pip install -r requirements-workflows.txt. Either update the workflows to use this requirements file, or remove it if it's intended only as documentation.

Suggested change
# Requirements for GitHub Actions workflows
# Used by various workflow files to generate configs and process results
pydantic>=2.0.0
pyyaml>=6.0.0
PyGithub>=2.0.0
matplotlib>=3.0.0
pytest>=7.0.0

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed requirements-workflows.txt as workflows install dependencies directly. (4dd647e)

cache: 'pip'

- name: Install dependencies
run: pip install pydantic pyyaml
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The workflow installs pydantic and pyyaml but the Python script only imports subprocess, json, and os. The pydantic dependency was previously installed inline before calling generate_sweep_configs.py as a subprocess, which still requires it. However, pyyaml doesn't appear to be needed in this job. Consider installing only the dependencies required by the subprocess call.

Suggested change
run: pip install pydantic pyyaml
run: pip install pydantic

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed to install only pydantic (removed pyyaml). (4dd647e)

Comment thread .github/workflows/collect-results.yml Outdated
cache: 'pip'

- name: Install dependencies
run: pip install matplotlib
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The previous version used pip install -q matplotlib with the -q (quiet) flag to reduce output verbosity. Consider retaining this flag for consistency with the original behavior and cleaner logs.

Suggested change
run: pip install matplotlib
run: pip install -q matplotlib

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Restored the -q flag for quiet output. (4dd647e)

@functionstackx
Copy link
Copy Markdown
Collaborator

@copilot Error: No file in /home/runner/work/InferenceMAX/InferenceMAX matched to [**/requirements.txt or **/pyproject.toml], make sure you have checked out the target repository

Co-authored-by: functionstackx <47992694+functionstackx@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Nov 9, 2025

@copilot Error: No file in /home/runner/work/InferenceMAX/InferenceMAX matched to [**/requirements.txt or **/pyproject.toml], make sure you have checked out the target repository

Removed cache: 'pip' from all workflows as it requires a requirements.txt or pyproject.toml file in the repository root. The workflows now use Python setup without caching. (4dd647e)

Copilot AI changed the title Move pip install to dedicated GitHub Actions steps with caching Move pip install to dedicated GitHub Actions steps Nov 9, 2025
Copy link
Copy Markdown
Collaborator

@cquil11 cquil11 left a comment

Choose a reason for hiding this comment

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

Seems reasonable

@cquil11 cquil11 merged commit b1df040 into main Nov 10, 2025
4 checks passed
@cquil11 cquil11 deleted the copilot/update-github-action-workflow branch November 10, 2025 17:01
functionstackx added a commit that referenced this pull request Nov 10, 2025
functionstackx added a commit that referenced this pull request Nov 10, 2025
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.

4 participants