Use uv-pre-commit to validate lockfile #6699

danielhollas · 2025-01-10T16:56:56Z

The previous uv hook that checked whether uv lockfile is up-to-date required the developer to have uv installed. Using the official uv-pre-commit hook, this is no longer the case. The hook also updates the lock automatically, instead of just checking its status.

Here's the output that I got when I bumped the mypy package in pyproject.toml

❯ pre-commit run uv-lock --all
uv-lock..................................................................Failed
- hook id: uv-lock
- files were modified by this hook

warning: `VIRTUAL_ENV=/home/hollas/.cache/pre-commit/repovyb9qrvk/py_env-python3.12` does not match the project environment path `.venv` and will be ignored
Resolved 263 packages in 2.50s
Updated mypy v1.13.0 -> v1.14.1

(the warning is a bit unfortunate interaction between uv and pre-commit, I opened astral-sh/uv-pre-commit#36 and asked if it could be hidden so that it is not confusing for devs)

danielhollas · 2025-01-10T16:57:27Z

.github/workflows/test-install.yml

@@ -45,9 +45,6 @@ jobs:
    - name: Install utils/ dependencies
      run: uv pip install --system -r utils/requirements.txt

-    - name: Validate uv lockfile


I think it's better to have this check only in one place.

codecov · 2025-01-10T17:39:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.09%. Comparing base (eba6954) to head (daa3fb5).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6699   +/-   ##
=======================================
  Coverage   78.09%   78.09%           
=======================================
  Files         564      564           
  Lines       42544    42544           
=======================================
  Hits        33219    33219           
  Misses       9325     9325

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

danielhollas · 2025-01-10T17:44:58Z

Hmm, this might be a problem, looks like pre-commit.ci blocks network requests. I think I've read about this in uv issue tracked

Using CPython 3.10.12 interpreter at: /usr/bin/python3
error: Failed to generate package metadata for `aiida-core==2.6.2.post0 @ editable+.`
  Caused by: Failed to resolve requirements from `build-system.requires`
  Caused by: No solution found when resolving: `flit-core>=3.4, <4`
  Caused by: Failed to fetch: `https://pypi.org/simple/flit-core/`
  Caused by: Could not connect, are you offline?
  Caused by: Request failed after 3 retries
  Caused by: error sending request for url (https://pypi.org/simple/flit-core/)
  Caused by: client error (Connect)
  Caused by: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: failed to lookup address information: Temporary failure in name resolution

EDIT: Indeed, this is a known issue when using dynamic metadata (such as version in our case) in `pyproject.toml: astral-sh/uv-pre-commit#35

danielhollas · 2025-01-15T14:57:51Z

This might get resolved in next uv version: astral-sh/uv#10622

.pre-commit-config.yaml

danielhollas · 2025-01-16T18:01:46Z

I notified the uv maintainers about the issue that blocks this PR and they already replied and it looks like it might not be difficult to fix. Other people are hitting the same issue as well so let wait a bit for the next release. astral-sh/uv#10689

danielhollas · 2025-01-18T11:59:56Z

.github/workflows/ci-code.yml

@@ -85,7 +85,7 @@ jobs:
      with:
        token: ${{ secrets.CODECOV_TOKEN }}
        name: aiida-pytests-py3.9
-        file: ./coverage.xml
+        files: ./coverage.xml


Unrelated change, this was throwing a warning in CI jobs.

khsrali · 2025-01-20T11:25:58Z

pyproject.toml

@@ -512,4 +512,8 @@ commands = molecule {posargs:test}
 """

 [tool.uv]
-required-version = ">=0.5.20"
+# NOTE: When you bump the minimum uv version, you also need to change it in:


Thanks a lot @danielhollas

Do we need to update also here? .github/workflows/test-install.yml (where you have astral-sh/[email protected])

Is it possible to simplify this, somehow?
Right now, this has to be updated manually in like 4 places...
Maybe one idea could be to have a hook on pre-commit to read the required version from pyproject.toml and if that doesn't match with other places (action.yml, .readthedocs.yml, etc), would automatically update them.

Do we need to update also here? .github/workflows/test-install.yml (where you have astral-sh/[email protected])

Good catch. In this case, I opted to simply always install the latest uv version, since we only use it to uv pip install utils/requirements.txt (i.e. no lockfile operations) so installing latest should be safe.

Is it possible to simplify this, somehow?

I've created #6721 as a follow-up, since it is not immediately clear how to improve this, and I don't want to block this PR since it is a clear improvement over status quo. I do agree it's not ideal, but also don't see it as a huge deal. Let's discuss in #6721.

khsrali

Thanks @danielhollas for opening the issue #6721
I agree and LGTM! :)

agoscinski · 2025-01-23T07:02:03Z

.github/workflows/test-install.yml

      with:
-        version: 0.5.x
+        version: latest


why do we use here latest and not version 0.5.22 as in the install-aiida-core action. Also in that action python-version: ${{ inputs.python-version }} is specified. Should we add here also a python-version: 3.11 ?

Please see: #6699 (comment)

We're only using the uv pip install interface here so we can keep things simple.

Also in that action python-version: ${{ inputs.python-version }} is specified. Should we add here also a python-version: 3.11 ?

In the actions case, specifying the python version is needed because it has a side-effect of automatically activating the virtual environment. Again, this is not needed here we can keep things simple and not duplicate the python version information.

I see thanks for the clarification

agoscinski

Thanks for the work!

There are two big advantages here: 1. Devs to have to have uv installed globally 2. We control uv version automatically.

agoscinski · 2025-01-24T13:45:07Z

pyproject.toml

+# .pre-commit-config.yaml
+# .github/actions/install-aiida-core/action.yml
+# .readthedocs.yml
+required-version = ">=0.5.21"


Sorry I did not notice this till now: Why do we accept also version 0.5.21 but bump in the other places the version to 0.5.22? I noticed that the version had be increased for some fixes. Are these fixes already included in 0.5.21?

Yes, all the fixes are in 0.5.21. We don't need to bump the minimum supported version unless really needed.

The previous uv hook that checked whether uv lockfile is up-to-date required the developer to have uv installed. Using the official uv-pre-commit hook, this is no longer the case. The hook also updates the lock automatically, instead of just checking its status. Bump minimum version of uv to `0.5.21` fix issue astral-sh/uv#10689 uv does run the build backend for projects with dynamic versioning. Bump astral-sh/setup-uv action to v5.2.1. Change in `.github/workflows/ci-code.yml` (`file` -> `files`) is related to fixing a warning in the CI.

danielhollas commented Jan 10, 2025

View reviewed changes

danielhollas marked this pull request as draft January 10, 2025 17:45

danielhollas commented Jan 15, 2025

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

danielhollas force-pushed the pre-commit-uv branch from 7583f79 to aae9e94 Compare January 15, 2025 22:13

unkcpz mentioned this pull request Jan 16, 2025

pre-commit: autorun uv lock if --check fails #6711

Closed

danielhollas force-pushed the pre-commit-uv branch 2 times, most recently from c7189a8 to 3766c25 Compare January 16, 2025 12:09

danielhollas mentioned this pull request Jan 16, 2025

Set minimum uv version in pyproject.toml #6714

Merged

danielhollas force-pushed the pre-commit-uv branch from 06bfeb3 to 7d77899 Compare January 16, 2025 17:56

danielhollas force-pushed the pre-commit-uv branch from 7e12fb3 to 87b098e Compare January 18, 2025 11:27

danielhollas marked this pull request as ready for review January 18, 2025 11:56

danielhollas requested review from unkcpz and agoscinski as code owners January 18, 2025 11:56

danielhollas commented Jan 18, 2025

View reviewed changes

danielhollas requested a review from khsrali January 18, 2025 12:00

khsrali reviewed Jan 20, 2025

View reviewed changes

danielhollas mentioned this pull request Jan 22, 2025

CI: How to manage uv version #6721

Open

danielhollas requested a review from khsrali January 22, 2025 02:09

khsrali approved these changes Jan 22, 2025

View reviewed changes

agoscinski reviewed Jan 23, 2025

View reviewed changes

danielhollas requested a review from agoscinski January 23, 2025 08:32

agoscinski approved these changes Jan 23, 2025

View reviewed changes

danielhollas added 4 commits January 24, 2025 14:14

Use uv-pre-commit to validate lockfile

ee5c84b

There are two big advantages here: 1. Devs to have to have uv installed globally 2. We control uv version automatically.

Bump setup-uv action to 5.2.1

6de1fd0

Fix coverage action warning

47e5cdd

Bump uv to 0.5.21

ebb2038

danielhollas added 3 commits January 24, 2025 14:14

Bump uv in .readthedocs.yml

7c2de7a

Install latest uv in test-install.yml

290818b

Bump uv to 0.5.22

daa3fb5

agoscinski force-pushed the pre-commit-uv branch from c5356f0 to daa3fb5 Compare January 24, 2025 13:14

agoscinski reviewed Jan 24, 2025

View reviewed changes

agoscinski merged commit 208d6a9 into aiidateam:main Jan 24, 2025
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use uv-pre-commit to validate lockfile #6699

Use uv-pre-commit to validate lockfile #6699

danielhollas commented Jan 10, 2025

danielhollas Jan 10, 2025

codecov bot commented Jan 10, 2025 •

edited

Loading

danielhollas commented Jan 10, 2025 •

edited

Loading

danielhollas commented Jan 15, 2025

danielhollas commented Jan 16, 2025

danielhollas Jan 18, 2025 •

edited

Loading

khsrali Jan 20, 2025 •

edited

Loading

danielhollas Jan 22, 2025

danielhollas Jan 22, 2025

khsrali left a comment

agoscinski Jan 23, 2025 •

edited

Loading

danielhollas Jan 23, 2025

danielhollas Jan 23, 2025

agoscinski Jan 23, 2025

agoscinski left a comment

agoscinski Jan 24, 2025 •

edited

Loading

danielhollas Jan 24, 2025

Use uv-pre-commit to validate lockfile #6699

Use uv-pre-commit to validate lockfile #6699

Conversation

danielhollas commented Jan 10, 2025

danielhollas Jan 10, 2025

Choose a reason for hiding this comment

codecov bot commented Jan 10, 2025 • edited Loading

Codecov Report

danielhollas commented Jan 10, 2025 • edited Loading

danielhollas commented Jan 15, 2025

danielhollas commented Jan 16, 2025

danielhollas Jan 18, 2025 • edited Loading

Choose a reason for hiding this comment

khsrali Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

danielhollas Jan 22, 2025

Choose a reason for hiding this comment

danielhollas Jan 22, 2025

Choose a reason for hiding this comment

khsrali left a comment

Choose a reason for hiding this comment

agoscinski Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

danielhollas Jan 23, 2025

Choose a reason for hiding this comment

danielhollas Jan 23, 2025

Choose a reason for hiding this comment

agoscinski Jan 23, 2025

Choose a reason for hiding this comment

agoscinski left a comment

Choose a reason for hiding this comment

agoscinski Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

danielhollas Jan 24, 2025

Choose a reason for hiding this comment

codecov bot commented Jan 10, 2025 •

edited

Loading

danielhollas commented Jan 10, 2025 •

edited

Loading

danielhollas Jan 18, 2025 •

edited

Loading

khsrali Jan 20, 2025 •

edited

Loading

agoscinski Jan 23, 2025 •

edited

Loading

agoscinski Jan 24, 2025 •

edited

Loading