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: Utilize uv lockfile for reproducible test environments #6640

Merged
merged 18 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 4 additions & 6 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -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
danielhollas marked this conversation as resolved.
Show resolved Hide resolved
environment.yml @aiidateam/dependency-manager
requirements*.txt @aiidateam/dependency-manager
pyproject.toml @aiidateam/dependency-manager
utils/dependency_management.py @aiidateam/dependency-manager
.github/workflows/dm.yml @aiidateam/dependency-manager
Copy link
Collaborator Author

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.

environment.yml @unkcpz @agoscinski
pyproject.toml @unkcpz @agoscinski
uv.lock @unkcpz @agoscinski
utils/dependency_management.py @unkcpz @agoscinski
37 changes: 17 additions & 20 deletions .github/actions/install-aiida-core/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ inputs:
required: false
# NOTE: Hard-learned lesson: we cannot use type=boolean here, apparently :-(
# https://stackoverflow.com/a/76294014
from-requirements:
description: Install aiida-core dependencies from pre-compiled requirements.txt file
# NOTE2: When installing from lockfile, aiida-core and its dependencies
# are installed in a virtual environment located in .venv directory.
# Subsuquent jobs steps must either activate the environment or use `uv run`
from-lock:
description: Install aiida-core dependencies from a uv lock file
default: 'true'
required: false

Expand All @@ -25,26 +28,20 @@ runs:
with:
python-version: ${{ inputs.python-version }}

- name: Install uv installer
run: curl --proto '=https' --tlsv1.2 -LsSf https://${{ env.UV_URL }} | sh
env:
UV_VERSION: 0.2.9
UV_URL: github.com/astral-sh/uv/releases/download/$UV_VERSION/uv-installer.sh
shell: bash
- name: Set up uv
uses: astral-sh/setup-uv@v4
with:
version: 0.5.6

- name: Install dependencies from requirements-py-*.txt
if: ${{ inputs.from-requirements == 'true' }}
run: uv pip install --system -r requirements/requirements-py-${{ inputs.python-version }}.txt
- name: Install dependencies from uv lock
if: ${{ inputs.from-lock == 'true' }}
# NOTE: We're asserting that the lockfile is up to date
# NOTE2: 'pre-commit' extra recursively contains other extras
# needed to run the tests.
run: uv sync --locked --extra pre-commit
Copy link
Collaborator Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we add the extra all that includes all extra packages to make it more explicit that we check all of them? I am not sure if there is a scenario where we will change something that is not included in pre-commit, but it is also not super clear that we choose pre-commit here to include all extras. At least I would add a comment

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 added a comment for now. I agree this is not ideal, I have some ideas but want to keep them for a separate PR since there's a lot going on here already.

Note that if we wanted to always install all the extras, we could just pass the --all-extras parameter to uv. But that would (among others) install the whole jupyter stack which seems wasteful.

shell: bash

- name: Install aiida-core
run: uv pip install --system ${{ env.NO_DEPS }} -e .${{ inputs.extras }}
env:
# Don't install dependencies if they were installed through requirements file AND
# if no extras are required.
#
# If this syntax looks weird to you, dear reader, know that this is
# GHA's way to do ternary operator. :-/
# https://docs.github.com/en/actions/learn-github-actions/expressions#example
NO_DEPS: ${{ (inputs.from-requirements == 'true' && inputs.extras == '') && '--no-deps' || '' }}
if: ${{ inputs.from-lock != 'true' }}
run: uv pip install --system -e .${{ inputs.extras }}
danielhollas marked this conversation as resolved.
Show resolved Hide resolved
shell: bash
43 changes: 5 additions & 38 deletions .github/workflows/ci-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,8 @@ env:

jobs:

check-requirements:

runs-on: ubuntu-latest
timeout-minutes: 5

steps:
- uses: actions/checkout@v4

- name: Set up Python 3.12
uses: actions/setup-python@v5
with:
python-version: '3.12'

- name: Install utils/ dependencies
run: pip install -r utils/requirements.txt

- name: Check requirements files
id: check_reqs
run: python ./utils/dependency_management.py check-requirements DEFAULT

- name: Create commit comment
if: failure() && steps.check_reqs.outputs.error
uses: peter-evans/commit-comment@v3
with:
path: pyproject.toml
body: |
${{ steps.check_reqs.outputs.error }}

Click [here](https://github.com/aiidateam/aiida-core/wiki/AiiDA-Dependency-Management) for more information on dependency management.

tests:

needs: [check-requirements]

runs-on: ubuntu-latest
timeout-minutes: 45

Expand Down Expand Up @@ -96,15 +64,16 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Setup environment
run: .github/workflows/setup.sh
# Note: The virtual environment in .venv was created by uv in previous step
run: source .venv/bin/activate && .github/workflows/setup.sh
Copy link
Member

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?

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 is created by uv sync automatically in the install-aiida-core action above.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we write it as comment or like this?

Suggested change
run: source .venv/bin/activate && .github/workflows/setup.sh
run: uv venv && source .venv/bin/activate && .github/workflows/setup.sh

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 wrote a comment about this both here and in actions/install-aiida-core/action.yml. See 7959c4d


- name: Run test suite
env:
AIIDA_TEST_PROFILE: test_aiida
AIIDA_WARN_v3: 1
# Python 3.12 has a performance regression when running with code coverage
# so run code coverage only for python 3.9.
run: pytest -n auto --db-backend psql -m 'not nightly' tests/ ${{ matrix.python-version == '3.9' && '--cov aiida' || '' }}
run: uv run pytest -n auto --db-backend psql -m 'not nightly' tests/ ${{ matrix.python-version == '3.9' && '--cov aiida' || '' }}

- name: Upload coverage report
if: matrix.python-version == 3.9 && github.repository == 'aiidateam/aiida-core'
Expand All @@ -118,7 +87,6 @@ jobs:

tests-presto:

needs: [check-requirements]
runs-on: ubuntu-latest
timeout-minutes: 20

Expand All @@ -139,12 +107,11 @@ jobs:
- name: Run test suite
env:
AIIDA_WARN_v3: 0
run: pytest -n auto -m 'presto' tests/
run: uv run pytest -n auto -m 'presto' tests/


verdi:

needs: [check-requirements]
runs-on: ubuntu-latest
timeout-minutes: 10

Expand All @@ -155,7 +122,7 @@ jobs:
uses: ./.github/actions/install-aiida-core
with:
python-version: '3.12'
from-requirements: 'false'
from-lock: 'false'

- name: Run verdi tests
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docs-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
with:
python-version: '3.9'
extras: '[docs,tests,rest,atomic_tools]'
from-requirements: 'false'
from-lock: 'false'

- name: Build HTML docs
id: linkcheck
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
with:
python-version: '3.11'
extras: '[pre-commit]'
from-requirements: 'false'
from-lock: 'false'

- name: Run pre-commit
run: pre-commit run --all-files || ( git status --short ; git diff ; exit 1 )
Expand Down
156 changes: 13 additions & 143 deletions .github/workflows/test-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
pull_request:
paths:
- environment.yml
- '**/requirements*.txt'
- pyproject.toml
- util/dependency_management.py
- .github/workflows/test-install.yml
Expand All @@ -14,7 +13,7 @@ on:

# https://docs.github.com/en/actions/using-jobs/using-concurrency
concurrency:
# only cancel in-progress jobs or runs for the current workflow - matches against branch & tags
# only cancel in-progress jobs or runs for the current workflow - matches against branch & tags
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

Expand All @@ -30,44 +29,24 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Set up Python 3.9
- name: Set up Python 3.11
uses: actions/setup-python@v5
with:
python-version: '3.9'

- name: Install utils/ dependencies
run: pip install -r utils/requirements.txt

- name: Validate
run: |
python ./utils/dependency_management.py check-requirements
python ./utils/dependency_management.py validate-all
unkcpz marked this conversation as resolved.
Show resolved Hide resolved

resolve-pip-dependencies:
# Check whether the environments defined in the requirements/* files are
# resolvable.

needs: [validate-dependency-specification]
if: github.repository == 'aiidateam/aiida-core'
runs-on: ubuntu-latest
timeout-minutes: 5

strategy:
fail-fast: false
matrix:
python-version: ['3.9', '3.10', '3.11', '3.12']
- name: Set up uv
uses: astral-sh/setup-uv@v4
with:
version: 0.5.6

steps:
- uses: actions/checkout@v4
- name: Install utils/ dependencies
run: uv pip install --system -r utils/requirements.txt

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Validate uv lockfile
run: uv lock --locked

- name: Resolve dependencies from requirements file.
run: |
pip install --dry-run --ignore-installed -r requirements/requirements-py-${{ matrix.python-version }}.txt
- name: Validate conda environment file
run: python ./utils/dependency_management.py validate-environment-yml

create-conda-environment:
# Verify that we can create a valid conda environment from the environment.yml file.
Expand Down Expand Up @@ -220,7 +199,7 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
extras: '[atomic_tools,docs,notebook,rest,tests,tui]'
from-requirements: 'false'
from-lock: 'false'

- name: Setup AiiDA environment
run: .github/workflows/setup.sh
Expand All @@ -230,112 +209,3 @@ jobs:
AIIDA_TEST_PROFILE: test_aiida
AIIDA_WARN_v3: 1
run: pytest -n auto --db-backend psql tests -m 'not nightly' tests/

- name: Freeze test environment
run: pip freeze | sed '1d' | tee requirements-py-${{ matrix.python-version }}.txt

# Add python-version specific requirements/ file to the requirements.txt artifact.
# This artifact can be used in the next step to automatically create a pull request
# updating the requirements (in case they are inconsistent with the pyproject.toml file).
- uses: actions/upload-artifact@v4
with:
name: requirements-py-${{ matrix.python-version }}.txt
path: requirements-py-${{ matrix.python-version }}.txt

# Check whether the requirements/ files are consistent with the dependency specification in the pyproject.toml file.
# If the check fails, warn the user via a comment and try to automatically create a pull request to update the files
# (does not work on pull requests from forks).

check-requirements:

needs: tests

runs-on: ubuntu-latest
timeout-minutes: 5

steps:
- uses: actions/checkout@v4

- name: Set up Python 3.9
uses: actions/setup-python@v5
with:
python-version: 3.9

- name: Install utils/ dependencies
run: pip install -r utils/requirements.txt

- name: Check consistency of requirements/ files
id: check_reqs
continue-on-error: true
run: python ./utils/dependency_management.py check-requirements DEFAULT --no-github-annotate

#
# The following steps are only executed if the consistency check failed.
#
- name: Create commit comment
if: steps.check_reqs.outcome == 'Failure' # only run if requirements/ are inconsistent
uses: peter-evans/commit-comment@v3
with:
token: ${{ secrets.GITHUB_TOKEN }}
path: pyproject.toml
body: |
The requirements/ files are inconsistent!

# Check out the base branch so that we can prepare the pull request.
- name: Checkout base branch
if: steps.check_reqs.outcome == 'Failure' # only run if requirements/ are inconsistent
uses: actions/checkout@v4
with:
ref: ${{ github.head_ref }}
clean: true

- name: Download requirements.txt files
if: steps.check_reqs.outcome == 'Failure' # only run if requirements/ are inconsistent
uses: actions/download-artifact@v4
with:
pattern: requirements-py-*
merge-multiple: true
path: requirements

- name: Commit requirements files
if: steps.check_reqs.outcome == 'Failure' # only run if requirements/ are inconsistent
run: |
git add requirements/*

- name: Create pull request for updated requirements files
if: steps.check_reqs.outcome == 'Failure' # only run if requirements/ are inconsistent
id: create_update_requirements_pr
continue-on-error: true
uses: peter-evans/create-pull-request@v7
with:
branch: update-requirements
commit-message: Automated update of requirements/ files.
title: Update requirements/ files.
body: |
Update requirements files to ensure that they are consistent
with the dependencies specified in the 'pyproject.toml' file.

Please note, that this pull request was likely created to
resolve the inconsistency for a specific dependency, however
other versions that have changed since the last update will
be included as part of this commit as well.

Click [here](https://github.com/aiidateam/aiida-core/wiki/AiiDA-Dependency-Management) for more information.

- name: Create PR comment on success
if: steps.create_update_requirements_pr.outcome == 'Success'
uses: peter-evans/create-or-update-comment@v4
with:
issue-number: ${{ github.event.number }}
body: |
I automatically created a pull request (#${{ steps.create_update_requirements_pr.outputs.pr_number }}) that adapts the
requirements/ files according to the dependencies specified in the 'pyproject.toml' file.

- name: Create PR comment on failure
if: steps.create_update_requirements_pr.outcome == 'Failure'
uses: peter-evans/create-or-update-comment@v4
with:
issue-number: ${{ github.event.number }}
body: |
Please update the requirements/ files to ensure that they
are consistent with the dependencies specified in the 'pyproject.toml' file.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel bad deleting this, but maintaining this seems to me me not worth the effort. Especially, since we don't update dependencies frequently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why do you feel bad? None of this is needed now when we have the lockfile, no?

Loading
Loading