Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install python packages into venvs across workflows. #640

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ScottTodd
Copy link
Member

@ScottTodd ScottTodd commented Dec 3, 2024

Many of these workflows are using persistent self-hosted runners, so it looks like they have been reusing the same system-wide Python environment between workflow runs (plus layer of caching on top). This switches to using venvs at ${{ github.workspace }}/.venv that should be ephemeral, giving us more explicit control over which packages are installed.

More work is planned as part of #584 to refactor these workflows further - replacing the package installs code like pip install --no-compile -r requirements.txt -r sharktank/requirements-tests.txt -e sharktank/ with a setup_venv.py script that uses dev/nightly/stable packages (from an appropriate source).

Good reading: https://adamj.eu/tech/2023/11/02/github-actions-faster-python-virtual-environments/

@@ -47,6 +47,8 @@ jobs:
uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0
with:
python-version: ${{matrix.version}}
- name: Create Python venv
run: python -m venv ${VENV_DIR}

- name: Cache Pip Packages
Copy link
Member Author

Choose a reason for hiding this comment

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

Notes to self:

Need to be deliberate with pip caching. I think I want the downloads to be cached, but not any installed packages.

Reference https://adamj.eu/tech/2023/11/02/github-actions-faster-python-virtual-environments/ and https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md#caching-packages.

Both of those approaches hash the requirements files as the cache key, but we have so many separate requirements files and we install our own dev and nightly packages on top, which are not pinned in any requirement files to be cached.

Some of that will hopefully be less consequential, if/when we switch to the setup_venv.py + uv setup style that I'm prototyping on #625.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe try landing tomorrow without any pip/venv caching, then figure out how to add it back carefully. Some extra CI time is worth the environment consistency.

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