-
Notifications
You must be signed in to change notification settings - Fork 192
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: Utilize uv lockfile for reproducible test environments #6640
base: main
Are you sure you want to change the base?
Conversation
5b73199
to
a3ef407
Compare
a3ef407
to
dff7bde
Compare
In this implementation, the hook will only check whether the lockfile is compatible with pyproject.toml, it will not try to automatically update it, since that should be decided by the developer.
- name: Install dependencies from uv lock | ||
# NOTE: We're also asserting that the lockfile is up to date | ||
if: ${{ inputs.from-lock == 'true' }} | ||
run: uv sync --locked --extra pre-commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: By default we're installing with pre-commit
optional dependencies (extras), which transitively includes other extras, see pyproject.toml I think this is the best solution to make things work for now.
@@ -2,9 +2,7 @@ | |||
# currently active dependency manager (DM) to trigger an automatic review | |||
# request from the DM upon changes. Please see AEP-002 for details: | |||
# https://github.com/aiidateam/AEP/tree/master/002_dependency_management | |||
setup.* @aiidateam/dependency-manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like @aiidateam/dependency-manager doesn't exist? Should I assign @agoscinski explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add both me and @agoscinski, or if you also want to maintain this ;) I think after this PR, the dependency management can be future simplified.
utils/dependency_management.py @aiidateam/dependency-manager | ||
.github/workflows/dm.yml @aiidateam/dependency-manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file does not exist.
@agoscinski @unkcpz I think this PR is mostly ready now, let me know what you think! The only thing missing is developer documentation: I would suggest moving the Dependency Management Wiki article to the repo in a separate PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6640 +/- ##
==========================================
+ Coverage 77.51% 77.94% +0.44%
==========================================
Files 560 563 +3
Lines 41444 41659 +215
==========================================
+ Hits 32120 32466 +346
+ Misses 9324 9193 -131 ☔ View full report in Codecov by Sentry. |
Sure, go ahead with a separate PR, thanks! |
@danielhollas nice, thanks a lot! would this be understood and accepted by |
@khsrali I tried it and it works without a problem. :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless they need to modify dependencies in pyproject.toml
I don't understand how this will work. I think if we run uv lock
from different machine, the lock file is generated using the environment where the command is run. Which means two people will have two different lock file generated. Is this leading to large number of lines change?
@@ -96,15 +64,15 @@ jobs: | |||
python-version: ${{ matrix.python-version }} | |||
|
|||
- name: Setup environment | |||
run: .github/workflows/setup.sh | |||
run: source .venv/bin/activate && .github/workflows/setup.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this .venv
created from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is created by uv sync
automatically in the install-aiida-core
action above.
uv should generate a universal lock file independent of your current python version or OS. At least that is my understanding, would be good if you can verify. I have generated the lock here on Linux and Python 3.12 Also you can try using the lock file I generated by running |
This PR removes the requirement files in
requirements/
folder and replaces them with a single universal lockfileuv.lock
. "universal" in this case means that it contains resolved package versions for all supported python versions and operating systems. More on the uv lockfiles can be found in their docs: https://docs.astral.sh/uv/concepts/projects/layout/#the-lockfileWhen modifying the dependencies in
pyproject.toml
, the lockfile needs to be updated by running(Note that using e.g.
uv add
,uv remove
oruv sync
updates the lockfile automatically)This relative simplicity compared to the previous custom solution allows for much easier development and I could delete a lot of custom YAML in CI. To check whether the lockfile is up to date, one can simply run
This command is run automatically as a pre-commit hook whenever
pyproject.toml
is changed.Note
Developers don't necessarily need to use the lockfile locally, nor are they required to have
uv
installed, unless they need to modify dependencies inpyproject.toml