Add support and testing for Python 3.12 and 3.13#4101
Add support and testing for Python 3.12 and 3.13#4101luckyh merged 4 commits intoavocado-framework:masterfrom
Conversation
|
Hello @pevogam, I had a PR for distutils, but it has some risks, what's your opinion of the |
Hi @PaulYuuu I think we should move forward from deprecated packages like this and comply with PEP 632. Can you add a reference/link to the distutils pull request you proposed? |
ah sorry, should be this one: #3920 |
pevogam
force-pushed
the
python3-updates
branch
from
54b4db0 to
22c3106
Compare
📝 WalkthroughWalkthroughCentralized Python versions in the CI workflow using a YAML anchor (adds 3.12 and 3.13), reused the anchor in job matrices, enabled Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Hi @PaulYuuu, the PR that we thought is a blocker for this is merged but I still get some problems here like So I assume we have to deprecate the setuptools as well. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
100-101: Remove deprecated egg build step and update distutils usageThe CI workflow’s “Build eggs” step uses
bdist_egg, which is deprecated and will fail on Python 3.12+ (where distutils has been removed). Additionally, the repository contains a direct import ofdistutils.version.LooseVersion, which must be replaced to ensure compatibility.• In .github/workflows/ci.yml (around lines 100–101):
- name: Build package run: python -m build - - name: Build eggs - run: python setup.py bdist_eggIf you still need to produce eggs for legacy consumers, gate this step to Python < 3.12 only:
- name: Build eggs (legacy) if: startsWith(matrix.python-version, '3.8') || startsWith(matrix.python-version, '3.9') || startsWith(matrix.python-version, '3.10') || startsWith(matrix.python-version, '3.11') run: python setup.py bdist_egg• In virttest/shared/scripts/virtio_console_guest.py (line 24):
- from distutils.version import LooseVersion # pylint: disable=W0611,E0611 + from packaging.version import parse as parse_version … - if LooseVersion(version) >= LooseVersion("1.0"): + if parse_version(version) >= parse_version("1.0"):Make sure to add
packagingas a dependency if it isn’t already. This change removes reliance on the removeddistutilsmodule in Python 3.12+.
🧹 Nitpick comments (5)
.github/workflows/ci.yml (5)
38-38: Avoid patch pin in matrix; use minor-only for stability and consistencyMixing a patch pin (“3.10.1”) with minor-only versions (“3.11”, “3.12”, “3.13”) is inconsistent and can break when that exact patch isn’t available on the runner. Prefer “3.10” here, like in package-build.
Apply this diff:
- python-version: ["3.8", "3.9", "3.10.1", "3.11", "3.12", "3.13"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]Optionally, ensure latest patch of each minor is used:
# under: uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} check-latest: true
85-85: De-duplicate version lists across jobs to prevent driftBoth jobs target the same minors. Consider centralizing via a YAML anchor so future updates touch one place.
Example:
jobs: travis-check: strategy: matrix: python-version: &py_versions ["3.8","3.9","3.10","3.11","3.12","3.13"] package-build: strategy: matrix: python-version: *py_versions
51-52: Upgrade pip before installing requirements to avoid resolver/ABI issues on 3.12/3.13Older pip/setuptools combos often choke on newest Python minors. Upgrade first, then install.
- - name: Install dependencies - run: pip install -r requirements-travis.txt + - name: Upgrade pip and install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-travis.txt
25-26: Apply the same pip upgrade in static-checks jobStatic checks run on the runner’s default Python (Ubuntu 24.04 → Python 3.12). Upgrade pip there too for consistency.
- - name: Install dependencies - run: pip install -r requirements-travis.txt + - name: Upgrade pip and install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-travis.txt
83-99: Optional: cache pip downloads to speed up CIactions/setup-python@v5 supports pip caching; it materially speeds up multi-version matrices.
Example:
- uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} cache: 'pip'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: fedora_40 AVOCADO_SRC:avocado-framework<104.0 SETUP:-m pip install . VT_TYPE:libvirt
- GitHub Check: fedora_40 AVOCADO_SRC:avocado-framework<104.0 SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:libvirt
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:qemu
- GitHub Check: fedora_40 AVOCADO_SRC:avocado-framework<104.0 SETUP:-m pip install . VT_TYPE:qemu
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:libvirt
- GitHub Check: fedora_40 AVOCADO_SRC:avocado-framework<104.0 SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:qemu
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install . VT_TYPE:libvirt
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install . VT_TYPE:qemu
- GitHub Check: Static checks
pevogam
force-pushed
the
python3-updates
branch
from
22c3106 to
72d191d
Compare
Hello @pevogam, Python >= 3.12 will not pre-install the setuptools module, see https://docs.python.org/3/whatsnew/3.12.html In short, to fix the setuptools problem in this PR, please consider adding |
pevogam
force-pushed
the
python3-updates
branch
from
72d191d to
59d615e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
requirements-travis.txt (2)
1-1: Tiny readability nit + optional context commentPurely cosmetic, but adding a space after the semicolon and a short comment helps future readers understand why this line exists.
- setuptools;python_version >= '3.12' + # Python ≥ 3.12 does not preinstall setuptools; needed for setup.py workflows + setuptools; python_version >= '3.12'
1-1: Consider setting a conservative lower bound and planning the move to PEP 517 buildsFor long‑term stability on 3.12/3.13, consider pinning a minimum setuptools version known to support those interpreters (e.g., >= 68/69+). Also, when you switch CI from “python setup.py …” to “python -m build”, you’ll want build tooling available.
Please verify the minimum setuptools version you want to require and whether “bdist_egg” is still needed in CI. If useful, here’s a minimal change that keeps current behavior but lays groundwork for the future:
- setuptools; python_version >= '3.12' + # runtime build backend for Python ≥ 3.12 + setuptools>=68; python_version >= '3.12' + # optional: when migrating CI to `python -m build` + build>=1.2; python_version >= '3.12' + wheel>=0.41; python_version >= '3.12'If you’d like, I can open a follow‑up PR to migrate the workflow to “python -m build” and drop egg builds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/ci.yml(2 hunks)requirements-travis.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
🧰 Additional context used
🪛 LanguageTool
requirements-travis.txt
[grammar] ~1-~1: There might be a mistake here.
Context: setuptools;python_version >= '3.12' Sphinx==1.3b1 # inspektor (static and st...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Static checks
🔇 Additional comments (2)
requirements-travis.txt (2)
1-1: LGTM: correct PEP 508 environment marker for setuptools on Python ≥ 3.12This is a sensible stop‑gap to fix the CI failure caused by setuptools no longer being bundled with Python ≥ 3.12. Nice and targeted.
2-11: Ensure compatibility of pinned tools with Python 3.12/3.13
Our local sanity-check on Python 3.11 failed to build the C extension fornetifaces==0.11.0(missing GCC/sysheaders) and couldn’t even create a virtualenv (ensurepip wasn’t available). This highlights that several of our legacy pins are brittle on modern interpreters—even if CI provides build tools, they’re at high risk of breakage on 3.12/3.13.• Gate legacy pins to older Python versions via environment markers and add up-to-date equivalents for ≥3.12.
• Alternatively, split requirements into per-purpose files (e.g.,docs/requirements.txt,lint/requirements.txt,test/requirements.txt) so you can modernize each slice without impacting the others.
• In your.github/workflows/ci.yml, confirm that:
- the
requirements-travis.txtinstall step runs before anypython setup.py …or project imports,- and that the CI runner includes
python3.x-venv,gcc, and other build dependencies needed for C extensions.Please manually verify that all matrix jobs for Python 3.12 and 3.13 install cleanly and pass in CI.
|
@pevogam , sorry I was in the wrong context, to For CI env, I don't think it's a problem that install setuptools even if it's installed(python<=3.11). But let's listen the voice from @richtja and @luckyh |
pevogam
force-pushed
the
python3-updates
branch
from
59d615e to
59ece03
Compare
|
Thanks for the quick feedback, I was just about to investigate this! Alright, pushed with the other suggested change, let's see. |
|
We could also do what the rabbit recommends: 
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
requirements.txt (1)
1-1: Avoid hard-pinning packaging and add a Python version marker to prevent breakage on older/guest environments.
- PEP 632 replacement in the guest script is good, but pinning
packaging==25.0globally may (a) increase resolver conflicts, and (b) try to installpackagingon Python 2 guests where this script may run, which will fail.- Prefer a compatible range and gate it to Python 3 only. If we implement a no-deps fallback in the guest script (see my other comment), we can even drop this dependency entirely.
Apply this diff:
-packaging==25.0 +packaging>=23,<26; python_version >= '3.9'If you decide to keep Python 3.8 support anywhere, adjust the marker accordingly.
Would you like me to open a follow-up to audit whether
requirements.txtis consumed on guests vs host-only? That determines if we can safely remove this line altogether..github/workflows/ci.yml (2)
87-87: Mirror the fromJSON-based matrix here as well.This alias is what triggers actionlint’s “alias node” and “unknown matrix property” diagnostics downstream.
Apply this diff:
- matrix: - python-version: *py_versions + matrix: + python-version: ${{ fromJSON(env.PY_VERSIONS) }}
102-104: Gate the legacy eggs step more simply and ensure setuptools is present.
- The current condition is verbose and includes 3.8, which isn’t in the matrix. Use an allowlist with fromJSON for clarity.
bdist_eggrequires setuptools; ensure it’s installed in this job to avoid the earlier 3.12+ missing-setuptools pitfall resurfacing in other images.Apply this diff to the condition:
- - name: Build eggs (legacy) - if: startsWith(matrix.python-version, '3.8') || startsWith(matrix.python-version, '3.9') || startsWith(matrix.python-version, '3.10') || startsWith(matrix.python-version, '3.11') + - name: Build eggs (legacy) + if: ${{ contains(fromJSON('["3.9","3.10","3.11"]'), matrix["python-version"]) }} run: python setup.py bdist_eggAnd (outside this hunk) update the install step to include setuptools:
- name: Install dependencies run: | python -m pip install --upgrade pip - pip install build + pip install build setuptoolsIf you want to be extra safe, we can also drop the legacy step entirely for 3.12/3.13 and keep only
python -m build, which already produces wheels and sdists in an isolated env.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/ci.yml(4 hunks)requirements.txt(1 hunks)virttest/shared/scripts/virtio_console_guest.py(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
87-87: "matrix values" section must be sequence node but got alias node with "" tag
(syntax-check)
103-103: property "python-version" is not defined in object type {}
(expression)
103-103: property "python-version" is not defined in object type {}
(expression)
103-103: property "python-version" is not defined in object type {}
(expression)
103-103: property "python-version" is not defined in object type {}
(expression)
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
51-52: Good: check-latest + pip cache.These options help keep the CI stable on patch releases and speed up installs. No change requested.
38-38: It looks likeactionlinttreated “never” as a file path because the flag needs two dashes. Please re-run the lint with the correct option and confirm there are no matrix-alias warnings:# Install actionlint if you haven’t already # Then from your repo root: actionlint --color never .github/workflows/ci.ymlOnce you’ve switched to a fromJSON-based matrix (as suggested), verify that this produces no warnings.
pevogam
force-pushed
the
python3-updates
branch
from
2f26346 to
5461a9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
38-38: Anchor is defined but not reused; prefer reusing across jobs or remove.You introduce &py_versions here but don’t reference it later. Since the package-build job tries to source versions elsewhere, either reuse this anchor there or drop it to avoid confusion.
Apply this to reuse the anchor in package-build:
- python-version: ${{ fromJSON(env.PY_VERSIONS) }} + python-version: *py_versions
51-52: Pip cache is good; consider pinning dependency paths for better cache hits.check-latest is fine; to improve cache keying, include the requirements file so cache busts when it changes.
with: python-version: ${{ matrix.python-version }} check-latest: true cache: 'pip' + cache-dependency-path: requirements-travis.txt
99-99: Include wheel to avoid build-tool gaps.Some PEP 517 builds still expect wheel present. It’s lightweight and avoids edge-case failures.
- pip install build setuptools + pip install build setuptools wheel
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(4 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 104-104: duplication of key "if" in mapping
(key-duplicates)
pevogam
force-pushed
the
python3-updates
branch
from
5461a9b to
9e710df
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
88-88: Correct reuse of the YAML anchor; resolves the earlier matrix expansion issue.Switching to
*py_versionshere fixes the previously noted fromJSON/env variable problem. This aligns both jobs to the same version set.
🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)
51-53: Setup-python caching looks good; minor tweaks can improve cache efficacy.
- pip cache key is derived from
cache-dependency-path; any deps installed outsiderequirements-travis.txt(e.g.,setuptools-rust, editable installs) won’t influence the cache key and may reduce hit rate.- Optional: include additional lock/manifest files if available, or add a secondary actions/cache around pip’s cache after checking out avocado-libs.
Example adjustments (only if such files exist in-repo):
with: python-version: ${{ matrix.python-version }} check-latest: true cache: 'pip' - cache-dependency-path: requirements-travis.txt + cache-dependency-path: | + requirements-travis.txt + pyproject.toml + setup.cfgIf you want higher determinism, consider dropping
check-latest: trueto reduce flakiness from latest patch bumps, though it’s fine to keep for catching regressions early.
54-56: Upgrade pip earlier to avoid resolver/build quirks on 3.12/3.13.You upgrade pip later inside the avocado-libs step, but the first
pip install -r requirements-travis.txtstill uses the runner’s default pip. Upgrading pip before this install improves reliability, especially with PEP 517/518 wheels on 3.13.- name: Install dependencies - run: pip install -r requirements-travis.txt + run: | + python -m pip install --upgrade pip + pip install -r requirements-travis.txt
93-100: Consider enabling pip caching in the package-build job as well.The build matrix will repeatedly download wheels across versions. Adding
cache: 'pip'here can trim minutes off cold runs.- name: Set up Python uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} + check-latest: true + cache: 'pip' + cache-dependency-path: | + pyproject.toml + setup.cfg
103-105: Eggs step gating is correct and fixes the duplicate-if lint error noted earlier.Limiting legacy
bdist_eggto 3.9–3.11 is reasonable. One caution: newer setuptools versions may deprecate or disablebdist_egg. If CI breaks in the future, consider pinningsetuptools<70for this step or dropping eggs entirely.Targeted pin (only for the eggs step), if needed:
- name: Build eggs (legacy) if: ${{ contains(fromJSON('["3.9","3.10","3.11"]'), matrix.python-version) }} - run: python setup.py bdist_egg + run: | + python -m pip install "setuptools<70" + python setup.py bdist_egg
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(4 hunks)
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
38-38: Good consolidation of Python versions via YAML anchor.Defining the matrix list once with &py_versions keeps jobs in sync and avoids env binding pitfalls. Nice cleanup.
100-100: Adding setuptools and wheel to build deps is the right call for 3.12+.This addresses the lack of preinstalled setuptools on 3.12+ and ensures wheels are generated. Thanks for incorporating the guidance from the discussion.
pevogam
force-pushed
the
python3-updates
branch
from
9e710df to
9f92faf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
38-38: Good call: central YAML anchor for Python versions (and reuse).Defining
&py_versionshere and reusing it across jobs simplifies future updates and fixes the earlier undefinedenv.PY_VERSIONSissue. Nicely done.
88-88: Matrix now reuses the anchor — resolves expansion errors.Switching to
*py_versionshere eliminates the prior matrix expansion failure and keeps both jobs in lockstep on supported versions.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
51-53: setup-python caching is configured correctly; minor consistency nit.
check-latest: true,cache: 'pip', andcache-dependency-path: requirements-travis.txtare all appropriate. As a small consistency tweak, preferpython -m pipeverywhere in this job to avoid PATH discrepancies.Apply within the existing “Install dependencies” step in this job:
- - name: Install dependencies - run: pip install -r requirements-travis.txt + - name: Install dependencies + run: python -m pip install -r requirements-travis.txt
97-99: Pip cache here likely won’t be effective; either add a dep file or drop cache.This job installs tools via an inline command (
pip install build setuptools wheel) with no requirements/lock file. Withoutcache-dependency-path, the cache key won’t reflect changes and may not provide value. Two options:
- Simpler (recommended): drop the cache for this job.
- Or: add a tiny requirements file and point the cache to it.
Option A — remove cache keys:
with: python-version: ${{ matrix.python-version }} - check-latest: true - cache: 'pip' + check-latest: trueOption B — introduce a build requirements file:
- - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install build setuptools wheel + - name: Prepare build requirements + run: printf "build\nsetuptools\nwheel\n" > requirements-build.txt + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install -r requirements-build.txt- with: - python-version: ${{ matrix.python-version }} - check-latest: true - cache: 'pip' + with: + python-version: ${{ matrix.python-version }} + check-latest: true + cache: 'pip' + cache-dependency-path: requirements-build.txt
105-106: Legacy eggs step is correctly gated; consider future removal or matrix-level gating.Single
if:fixes the duplicate-key issue noted earlier. If eggs are truly legacy, consider moving the constraint intostrategy(exclude/include) or planning deprecation to simplify the workflow.If eggs are still required on 3.8, adjust the list accordingly:
- if: ${{ contains(fromJSON('["3.9","3.10","3.11"]'), matrix.python-version) }} + if: ${{ contains(fromJSON('["3.8","3.9","3.10","3.11"]'), matrix.python-version) }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(4 hunks)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
102-102: Build tooling install looks correct for 3.12/3.13.Installing
build setuptools wheelcovers both wheel builds and the legacybdist_eggstep that follows (setuptools). No issues here.
pevogam
force-pushed
the
python3-updates
branch
from
9f92faf to
d0b6fa8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
51-53: Cache is good; broaden cache-dependency-path for robust invalidation.Including only requirements-travis.txt won’t invalidate the pip cache when build inputs change (e.g., pyproject.toml, setup.cfg, setup.py). Recommend listing them explicitly.
with: python-version: ${{ matrix.python-version }} check-latest: true - cache: 'pip' - cache-dependency-path: requirements-travis.txt + cache: 'pip' + cache-dependency-path: | + requirements-travis.txt + pyproject.toml + setup.cfg + setup.py
97-97: check-latest enabled — OK, just note reproducibility trade-off.This pulls the newest patch for each minor on every run. If you ever need fully reproducible CI, consider pinning patch versions in the matrix. Otherwise, this is fine.
101-101: Prefer invoking pip via the interpreter.Minor but safer across environments; keeps installs tied to the selected Python.
- pip install build setuptools wheel + python -m pip install build setuptools wheel
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
106-106: could not parse as YAML: yaml: line 106: mapping values are not allowed in this context
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 108-108: no new line character at the end of file
(new-line-at-end-of-file)
[error] 106-106: syntax error: mapping values are not allowed here
(syntax)
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
38-38: Centralized Python versions via YAML anchor — nice.Defining &py_versions once and reusing it improves readability and avoids drift across jobs.
88-88: *Reusing py_versions in package-build matrix — aligned and DRY.Referencing the earlier anchor here resolves the prior “undefined env.PY_VERSIONS” concern and keeps both jobs in sync.
pevogam
force-pushed
the
python3-updates
branch
from
d0b6fa8 to
193c3c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
104-106: Legacy eggs gating is correct; duplicate-if issue resolvedThe step now has a single, valid if: predicate and is correctly limited to 3.9–3.11, avoiding Python 3.12+ where setuptools is not preinstalled and distutils is removed. Looks good.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
97-101: Mirror pip caching in package-build for consistency and speedOptional, but aligning this job with the travis-check caching will cut repeated downloads across the matrix. Also safe to keep check-latest here.
Suggested adjustments:
- - name: Set up Python + - name: Set up Python uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} check-latest: true + cache: 'pip' + cache-dependency-path: | + requirements-travis.txt + **/pyproject.toml + **/setup.cfgNote: the globbed files are optional; if they don’t exist, caching still works using requirements-travis.txt.
56-71: Double-check avocado-libs build deps on 3.13Minor verification: the pinned setuptools-rust==1.1.2 plus editable install of avocado-libs should work on 3.12/3.13, but if upstream bumps minimum versions, this may break first in 3.13. Keep an eye on this in CI; if it flakes, consider loosening the pin or moving to pyproject-managed build deps.
104-106: Optional: derive eggs gating from a single source to avoid driftIf you adopt env.PY_VERSIONS, you can also centralize the eggs gate to avoid forgetting to update both lists. For example, define an env for eggable versions:
Add (outside this range):
env: PY_EGG_VERSIONS: '["3.9","3.10","3.11"]'Then adjust the condition:
- if: ${{ contains(fromJSON('["3.9","3.10","3.11"]'), matrix.python-version) }} + if: ${{ contains(fromJSON(env.PY_EGG_VERSIONS), matrix.python-version) }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
88-88: "matrix values" section must be sequence node but got alias node with "" tag
(syntax-check)
105-105: property "python-version" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Static checks
- GitHub Check: fedora_40 AVOCADO_SRC: SETUP:-m pip install PYPI_UPLOAD/*.whl VT_TYPE:qemu
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
51-53: Good use of setup-python caching and check-latestcheck-latest plus pip cache and dependency path on requirements-travis.txt is a solid speedup for the travis-check job. No issues.
|
Alright, @PaulYuuu I think now I made both you and the rabbit bot happy (the latter happy enough as it cannot be easily satiated) so let me know if the final commits and changes look good enough to you. |
|
Oh, @pevogam, please drop the python 3.8 commit, the current master branch already removed it. |
|
Hi @PaulYuuu, I just fetched and rebased but I don't see it being removed, can you point me to exact commit hash? |
Should be b6ba9b7, and you can check the file from web to view. |
Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
Limit the egg build to earlier python versions only. Also add wheel as it is lightweight and can handle edge-case failures. Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
pevogam
force-pushed
the
python3-updates
branch
from
193c3c5 to
f55c8da
Compare
|
I see, the time the pull request added 3.12 and 3.13 was before the 3.8 was dropped on master so this change ended up reintroducing the 3.8. Nice catch, must be fixed now. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
99-101: Use python -m pip consistently.Minor consistency nit: prefer python -m pip for all pip invocations.
Apply:
- python -m pip install --upgrade pip - pip install build setuptools wheel + python -m pip install --upgrade pip + python -m pip install build setuptools wheel
94-98: Optional: add pip cache to package-build for speed.Mirroring travis-check, caching pip here speeds builds and is safe.
- name: Set up Python uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - check-latest: true + check-latest: true + cache: 'pip' + # Cache invalidates when build tooling changes + cache-dependency-path: | + pyproject.toml + setup.cfg + setup.py + requirements*.txt
88-88: Heads-up: actionlint 1.7.7 errors on YAML anchors inmatrix.python-versionactionlint reports on line 88:
.github/workflows/ci.yml:88:25: "matrix values" section must be sequence node but got alias node with "" tag [syntax-check]Options (pick one):
- Inline the Python versions list in this job instead of using the
*py_versionsanchor.- Upgrade actionlint to a version that tolerates aliases in matrix values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ci.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pevogam
PR: avocado-framework/avocado-vt#4101
File: .github/workflows/ci.yml:36-39
Timestamp: 2025-08-25T15:08:02.212Z
Learning: For GitHub Actions workflows in avocado-vt, pevogam prefers using YAML anchors (&py_versions and *py_versions) over fromJSON(env.PY_VERSIONS) for centralizing Python version matrices. Do not repeatedly suggest the fromJSON approach when YAML anchors are being used.
📚 Learning: 2025-08-25T15:08:02.212Z
Learnt from: pevogam
PR: avocado-framework/avocado-vt#4101
File: .github/workflows/ci.yml:36-39
Timestamp: 2025-08-25T15:08:02.212Z
Learning: For GitHub Actions workflows in avocado-vt, pevogam prefers using YAML anchors (&py_versions and *py_versions) over fromJSON(env.PY_VERSIONS) for centralizing Python version matrices. Do not repeatedly suggest the fromJSON approach when YAML anchors are being used.
Applied to files:
.github/workflows/ci.yml
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
88-88: "matrix values" section must be sequence node but got alias node with "" tag
(syntax-check)
105-105: property "python-version" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Static checks
🔇 Additional comments (4)
.github/workflows/ci.yml (4)
38-38: Good centralization of Python versions with a YAML anchor.Matches the repo preference and keeps both matrices in sync.
51-53: Setup-python caching and check-latest look good.cache-dependency-path points to requirements-travis.txt, which should give deterministic cache busting here.
97-97: check-latest on package-build is fine.Consistent with the other job.
104-106: Eggs step gating verified
Step triggers only for Python 3.9–3.11 as intended. No changes required.
Hi @pevogam and @PaulYuuu, FYI we do the same on avocado side, see. |
|
Merging this, thanks to all! |
4 participants
Summary by CodeRabbit