8000 Cache pip downloads in CI by Avasam · Pull Request #9192 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Cache pip downloads in CI #9192

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 15, 2022
Merged

Cache pip downloads in CI #9192

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
066fd23
Cache pip downloads
Avasam Nov 13, 2022
9313f2e
Merge branch 'main' of https://github.com/python/typeshed into cache-…
Avasam Nov 13, 2022
61cd165
cache-dependency-path mypy_primer
Avasam Nov 13, 2022
fc69554
PR comments
Avasam Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/daily.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ jobs:
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- name: Update pip
if: matrix.os == "macos-11"
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this line? I usually update pip on all envs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Usually, pip is always updated and included in runner images. Because of this, the latest version of pip is usually installed already.

actions/setup-python#506 (comment)

I can only assume python -m pip install -U pip was done because of "macos-11". As the only times that command is ran is when also using "macos-11. So only running it on that OS is a small optimization for the others.

Copy link
Member

Choose a reason for hiding this comment

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

Given that we have PIP_DISABLE_PIP_VERSION_CHECK: 1 set as an environment variable in this workflow, we probably don't need this step at all. It usually isn't essential to have the very latest version of pip installed; the only significant benefit from upgrading pip in CI is that you don't get the annoying "WARNING: a newer version of pip is available, you should upgrade NOW" message with every CI job.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't even notice PIP_DISABLE_PIP_VERSION_CHECK. Should I remove the step? Leave it as it was before?

Copy link
Member
@AlexWaygood AlexWaygood Nov 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I reckon you can remove it. There's other workflows we have in this repo where we don't have it as a step, and removing the step fits with the theme of this PR ("speed up CI by getting pip to do less work").

run: python -m pip install -U pip
- name: Install dependencies
run: pip install $(grep tomli== requirements-tests.txt) $(grep mypy== requirements-tests.txt)
Expand All @@ -55,6 +58,10 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.9"
cache: 'pip'
cache-dependency-path: |
requirements-tests.txt
Copy link
Member

Choose a reason for hiding this comment

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

Should this contain METADATA.toml files? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When we'll be able to specify non-types dependencies, are they going to be specified in METADATA.toml ? If so, then yes cache-dependency-path should contain METADATA.toml for stubtest jobs.

Copy link
Member

Choose a reason for hiding this comment

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

When we'll be able to specify non-types dependencies, are they going to be specified in METADATA.toml ?

I think that's the idea, yup!

stubs/**/@tests/requirements-stubtest.txt
- name: Install dependencies
run: pip install -r requirements-tests.txt
- name: Run stubtest
Expand Down Expand Up @@ -86,6 +93,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: stub_uploader/requirements.txt
- name: Run tests
run: |
cd stub_uploader
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/mypy_primer.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: |
typeshed_to_test/requirements-tests.txt
typeshed_to_test/stubs/**/@tests/requirements-stubtest.txt
typeshed_to_test/stubs/**/METADATA.toml
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this will actually speed anything up. The only thing this workflow directly pip-installs is mypy_primer, which has 0 dependencies. mypy_primer will then dynamically install loads of things during the course of the script's execution, but I don't think it'll ever install anything from typeshed's requirements-tests.txt file, any METADATA.toml files, or any requirements-stubtest.txt files.

Maybe just get rid of the changes to this workflow file, for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would be more sensible to base the cache key on mypy_primer.py. But I can't really specify that. mypy_primer downloads so much it would make sense to cache those. But then how do we specify that the cache should be invalidated? Here I tried to target everything that may suggest a need to invalidate (ie version bump or dependency change of the stub).

Anyway, this can be reconsidered, I'll revert for now.

- name: Install dependencies
run: pip install git+https://github.com/hauntsaninja/mypy_primer.git
- name: Run mypy_primer
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/stubsabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- name: git config
run: |
git config --global user.name stubsabot
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/stubtest_stdlib.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ jobs:
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- name: Update pip
if: matrix.os == "macos-11"
run: python -m pip install -U pip
- name: Install dependencies
run: pip install $(grep mypy== requirements-tests.txt)
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/stubtest_third_party.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.9"
cache: 'pip'
cache-dependency-path: |
requirements-tests.txt
stubs/**/@tests/requirements-stubtest.txt
- name: Install dependencies
run: pip install -r requirements-tests.txt
- name: Run stubtest
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- run: pip install -r requirements-tests.txt
- run: ./tests/check_consistent.py

Expand All @@ -50,6 +52,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- run: pip install -r requirements-tests.txt
- run: flake8

Expand All @@ -61,6 +65,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- run: pip install -r requirements-tests.txt
- run: ./tests/pytype_test.py --print-stderr

Expand All @@ -77,6 +83,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- run: pip install -r requirements-tests.txt
- run: python ./tests/mypy_test.py --platform=${{ matrix.platform }} --python-version=${{ matrix.python-version }}

Expand All @@ -88,6 +96,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
- run: pip install -r requirements-tests.txt
- run: python ./tests/regr_test.py --all

Expand Down Expand Up @@ -141,6 +151,8 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.10"
cache: 'pip'
cache-dependency-path: stub_uploader/requirements.txt
- name: Run tests
run: |
cd stub_uploader
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/typecheck_typeshed_code.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: 3.9
python-version: "3.9"
cache: 'pip'
cache-dependency-path: requirements-tests.txt
Copy link
Member

Choose a reason for hiding this comment

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

          python-version: "3.9"
          cache: 'pip'
          cache-dependency-path: requirements-tests.txt

Consider this as a rant about yaml: these three lines have three different quotes 😒

Copy link
Collaborator Author
@Avasam Avasam Nov 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

😅 What's the preferred quote style between " and ' here? Normally I use the redhat.vscode-yaml plugin which formats to "(non-configurable). But I've disabled all my auto-formatters on non-python files in this project.

Copy link
Member

Choose a reason for hiding this comment

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

What's the preferred quote style between " and ' here?

I don't think we have one, and I for one don't care at all, as long as it works :)

- run: pip install -r requirements-tests.txt
- run: python ./tests/typecheck_typeshed.py --platform=${{ matrix.platform }}
0