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

ci: enable uv cache, use astral-sh official action #956

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

aidoskanapyanov
Copy link
Contributor

@aidoskanapyanov aidoskanapyanov commented Sep 12, 2024

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below.

Also added cache-dependency-glob pattern to revalidate the cache whenever the requirements-dev.txt file is changed. I hope this will work, gotta test it :)

@aidoskanapyanov
Copy link
Contributor Author

This is how uv cached the dependencies, I also added a python-version suffix to distinguish between versions:
https://github.com/narwhals-dev/narwhals/actions/caches

Shall we trigger CI rerun and see if it cuts the install-reqs step?

@FBruzzesi
Copy link
Member

This is how uv cached the dependencies, I also added a python-version suffix to distinguish between versions: https://github.com/narwhals-dev/narwhals/actions/caches

Shall we trigger CI rerun and see if it cuts the install-reqs step?

I re-triggered for python 3.12 here

@FBruzzesi FBruzzesi changed the title ci: enable uv cache, use astral-sh official aciton ci: enable uv cache, use astral-sh official action Sep 13, 2024
@MarcoGorelli
Copy link
Member

did it work? shall we ship it?

@FBruzzesi
Copy link
Member

did it work? shall we ship it?

I honestly could not spot much of the difference, but still would prefer to use the official action rather then a manual installation.
From the uv github section, it seems we could use uv to install run tests as well (happy to keep it as a followup)

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

agree - let's do this then, thanks @aidoskanapyanov !

@MarcoGorelli MarcoGorelli merged commit db383a4 into narwhals-dev:main Sep 13, 2024
26 checks passed
@aidoskanapyanov
Copy link
Contributor Author

aidoskanapyanov commented Sep 24, 2024

image

I pulled up statistics for install-reqs step in pytest-38 (3.8, ubuntu-latest) job in PyTest workflow for the last 60-ish days via gh api.
And it turns out there's really no difference in using cache for installing dependencies. If anything this may have slowed down the run times :/

@aidoskanapyanov aidoskanapyanov deleted the enh_enable_uv_cache branch September 24, 2024 12:35
@FBruzzesi
Copy link
Member

FBruzzesi commented Sep 24, 2024

Wow that some graph! Thanks for taking such an in depth look at this!

I guess that uv is faster than building, checking and grabbing from a cache then πŸ˜‚ Should we revert this one?

@aidoskanapyanov
Copy link
Contributor Author

aidoskanapyanov commented Sep 24, 2024

Yeah, I think we need to revert it, installing from https://astral.sh/uv/install.sh looked fine. Seems like a pretty permanent url to me πŸ˜‚
Also, cache entries are piling up with each CI run in https://github.com/narwhals-dev/narwhals/actions/caches.
Some of them are being used, but I don't think it makes much of a difference in performace.

@aidoskanapyanov
Copy link
Contributor Author

aidoskanapyanov commented Sep 24, 2024

If anything this may have slowed down the run times

Could also be that we've added 3 new dev dependencies since then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enh]: Use offical astral-sh/setup-uv@v2 github action and enable caching in the CI
3 participants