-
Notifications
You must be signed in to change notification settings - Fork 159
Speed up CI by parallelizing pulp-cli livetests and using uv. #7954
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,9 +29,16 @@ jobs: | |
| - uses: "actions/setup-python@v6" | ||
| with: | ||
| python-version: "3.12" | ||
| - name: "Install uv" | ||
| uses: "astral-sh/setup-uv@v7" | ||
| with: | ||
| enable-cache: true | ||
| - name: "Configure uv to use system Python" | ||
| run: | | ||
| echo "UV_SYSTEM_PYTHON=1" >> "$GITHUB_ENV" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to set this globally rather than needing to specify it everywhere
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could. Currently this comes from a |
||
| - name: "Install python dependencies" | ||
| run: | | ||
| pip install towncrier | ||
| uv pip install towncrier | ||
| - name: "Build changelog" | ||
| run: | | ||
| towncrier build --yes --version 4.0.0.ci | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,7 +117,7 @@ cmd_stdin_prefix bash -c "cat > /tmp/unittest_requirements.txt" < unittest_requi | |
| cmd_stdin_prefix bash -c "cat > /tmp/functest_requirements.txt" < functest_requirements.txt | ||
| cmd_stdin_prefix bash -c "cat > /tmp/bindings_requirements.txt" < bindings_requirements.txt | ||
| cmd_stdin_prefix bash -c "cat > /tmp/bindings_constraints.txt" < bindings_constraints.txt | ||
| cmd_prefix pip3 install -r /tmp/unittest_requirements.txt -r /tmp/functest_requirements.txt -r /tmp/bindings_requirements.txt -c /tmp/bindings_constraints.txt | ||
| cmd_prefix uv pip install -r /tmp/unittest_requirements.txt -r /tmp/functest_requirements.txt -r /tmp/bindings_requirements.txt -c /tmp/bindings_constraints.txt | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like there are a few installs, not just the one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few more below also |
||
|
|
||
| CERTIFI=$(cmd_prefix python3 -c 'import certifi; print(certifi.where())') | ||
| cmd_prefix bash -c "cat /etc/pulp/certs/pulp_webserver.crt >> '$CERTIFI'" | ||
|
|
@@ -170,12 +170,15 @@ export PULP_FIXTURES_URL="http://pulp-fixtures:8080" | |
| # some pulp-cli tests use the api root envvar | ||
| export PULP_API_ROOT="$(EDITOR=cat pulp config edit 2>/dev/null | awk -F'"' '/api_root/{print $2; exit}')" | ||
| pushd ../pulp-cli | ||
| # livetests are parallel-safe (pytest-xdist). | ||
| CLI_MARK="live and (pulpcore or pulp_file or pulp_certguard)" | ||
| if [[ -f "test_requirements.txt" ]] | ||
| then | ||
| pip install -r test_requirements.txt | ||
| pytest -v tests -m "pulpcore or pulp_file or pulp_certguard" | ||
| uv pip install -r test_requirements.txt | ||
| pytest -v tests -m "${CLI_MARK}" -n 8 | ||
| else | ||
| PULP_CA_BUNDLE="/usr/local/share/ca-certificates/pulp_webserver.crt" make livetest PYTEST_MARK="live and (pulpcore or pulp_file or pulp_certguard)" | ||
| PULP_CA_BUNDLE="/usr/local/share/ca-certificates/pulp_webserver.crt" \ | ||
| uv run pytest -v tests pulp-glue/tests -m "${CLI_MARK}" -n 8 | ||
| fi | ||
| popd | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Run pulp-cli livetests in parallel with pytest-xdist, and use uv instead of pip in CI. |
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.
Is this actually correct?
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 might have been at one point. Think the main reason we want there to be only one install is so that dependency resolution is more reliable, but I can't remember.
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 is for pip that ignores already installed packages when called a second time.
I think uv is addressing this in some way. But we need to be careful.
(In some cases you end up with broken packages, in others you end up with package versions that weren't the ones you set out to test in the first place.)
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.
But I think the "full solution" is not to use "uv pip" but to go "uv" all the way.