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

Use uv to install packages #1369

Merged
merged 4 commits into from
Aug 14, 2024
Merged

Use uv to install packages #1369

merged 4 commits into from
Aug 14, 2024

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Aug 14, 2024

On a recent PR, re-packing the cache took more time than the actual job. GHA runners have fast internet connections and we don't use many packages, it probably isn't worth using the cache. Also, we can switch to using uv for faster times.

Timings:

Timings Old New
Run actions/checkout@v4 ~0s ~0s
Run actions/setup-python@v4 ~0s ~0s
Install uv N/A 1s
Build docs 17s 12s
Link check 1m 30s 1m 22s
Post Run actions/setup-python@v4 2m 14s 0s
Post Run actions/checkout@v4 0s 0s
Total 4m 4s 1m 37s

A


📚 Documentation preview 📚: https://cpython-devguide--1369.org.readthedocs.build/

@hugovk
Copy link
Member

hugovk commented Aug 14, 2024

2m12 is a long time for repacking. I've seen actions/cache be much slower in other repos than it used to be lately, wonder what's up there.

I'd be curious to see how it works with https://github.com/hynek/setup-cached-uv, could we try it in a couple of builds?

Even though uv might be quicker with no cache, but it would be good to ease the load on PyPI (or at least set an example to encourage it) if it's not too slow.

2021: 900 TB/day https://dustingram.com/articles/2021/04/14/powering-the-python-package-index-in-2021/

2024: 4 PB/day

image

@AA-Turner
Copy link
Member Author

Even though uv might be quicker with no cache, but it would be good to ease the load on PyPI (or at least set an example to encourage it) if it's not too slow.

Instinctively I'm hesitant to as I'm not sure it's worth the extra complexity -- I assume that the vast majority of that bandwidth comes from very heavy packages like tensorflow. I will have a look, though.

A

Comment on lines 19 to 24
- name: Install uv
run: >
curl --no-progress-meter --location --fail
--proto '=https' --tlsv1.2
"https://astral.sh/uv/install.sh"
| sh
Copy link
Member

Choose a reason for hiding this comment

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

I think it's less complex? (or the complexity is hidden ;) aka managed for us)

Suggested change
- name: Install uv
run: >
curl --no-progress-meter --location --fail
--proto '=https' --tlsv1.2
"https://astral.sh/uv/install.sh"
| sh
- name: Install uv
uses: hynek/setup-cached-uv@v2

Copy link
Member Author

Choose a reason for hiding this comment

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

Explicit is better than implicit! More to the point, hynek/setup-cached-uv@v2 fetches from a third-party domain without being clear about it, which I'm not a massive fan of (that action also doesn't have the proto or tls flags which protect against downgrade attacks). We could change the curl command here to the suggested curl -LsSf https://astral.sh/uv/install.sh | sh, but I think it is better to use long flags (-sS ~= --no-progress-meter, -L == --location, -f == --fail) and have the downgrade protection.

Taking the following commits as examples, there seems to be a c. 0.07s-0.1s speed-up in the resolving step with the action and a 0.04s-0.06s speed-up in the preparing step.

Personally, I would rather be explicit about the install (especially as we are piping-to-sh installing a third-party script) than loose the ~0.15s speed-up. But perhaps having the complexity hidden is worth it on its own.


beb7956 (baseline, initialises cache)

Using Python 3.12.4 interpreter at: /opt/hostedtoolcache/Python/3.12.4/x64/bin/python3
Creating virtualenv at: ./venv
Resolved 43 packages in 195ms
Prepared 43 packages in 480ms
Installed 43 packages in 49ms

2f19ac5 (first with cache)

Using Python 3.12.4 interpreter at: /opt/hostedtoolcache/Python/3.12.4/x64/bin/python3
Creating virtualenv at: ./venv
Resolved 43 packages in 127ms
Prepared 43 packages in 418ms
Installed 43 packages in 47ms

ad74301 (empty commit)

Using Python 3.12.4 interpreter at: /opt/hostedtoolcache/Python/3.12.4/x64/bin/python3
Creating virtualenv at: ./venv
Resolved 43 packages in 95ms
Prepared 43 packages in 437ms
Installed 43 packages in 59ms

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 fine with using it, it's maintained by a core developer who also maintains lots of popular packages, so I trust care is being taken.

It would be nice to outsource uv installation: there's a bunch of options for that curl command that might need changing later.

Plus this workflow is not creating any build artifacts used elsewhere: we're building docs, checking links, and throwing the whole environment away.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough!

@AA-Turner AA-Turner merged commit e8db121 into python:main Aug 14, 2024
4 checks passed
@AA-Turner AA-Turner deleted the no-cache branch August 14, 2024 17:51
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