CI: Modernize OS runner and Python versions #245
Conversation
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughUpdated CI workflows and repository metadata: bumped Python toolchain versions, reworked test matrix and triggers, changed Dependabot schedule to daily, adjusted release workflow user, and added Python 3.14 classifier and release extras in packaging config. (48 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #245 +/- ##
=======================================
Coverage 92.49% 92.49%
=======================================
Files 27 27
Lines 1825 1825
=======================================
Hits 1688 1688
Misses 137 137
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
34-36: Clarify the OS and Python 3.7 strategy.The OS matrix is now ubuntu-latest only, with an explicit exception for ubuntu-22.04 + Python 3.7. This pairing is unusual:
- Python 3.7 is EOL (June 2023) and deprecated
- ubuntu-22.04 ships with Python 3.10, not 3.7
- The explicit pairing suggests deliberate support for 3.7, but unclear why only on this OS
If Python 3.7 support has been dropped, remove this pairing. If it's still needed, clarify the rationale and consider documenting why.
Also applies to: 41-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish.yml(1 hunks).github/workflows/test.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/test.yml (1)
63-63: Quoting the editable install flag is a good fix.The change from
--editable .[test,develop]to--editable='.[test,develop]'prevents shell interpretation of the bracket syntax. This ensures the extras are correctly parsed.
| os: [ | ||
| 'ubuntu-latest', | ||
| ] | ||
| python-version: [ | ||
| '3.7', '3.13', | ||
| 'pypy-3.9', 'pypy-3.10', | ||
| '3.14', | ||
| 'pypy-3.11', | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for Python version requirements
echo "=== setup.cfg (if exists) ==="
if [ -f setup.cfg ]; then
cat setup.cfg | grep -A 5 -E "(python_requires|Programming Language)"
else
echo "File not found"
fi
echo -e "\n=== pyproject.toml (if exists) ==="
if [ -f pyproject.toml ]; then
cat pyproject.toml | grep -A 3 -E "(requires-python|requires_python)"
else
echo "File not found"
fi
echo -e "\n=== setup.py (if exists) ==="
if [ -f setup.py ]; then
cat setup.py | grep -A 2 "python_requires" || echo "python_requires not found"
fi
echo -e "\n=== Current .github/workflows/test.yml matrix (lines 34-43) ==="
if [ -f .github/workflows/test.yml ]; then
sed -n '34,43p' .github/workflows/test.yml
fiRepository: grafana-toolbox/grafana-client
Length of output: 1238
Correct the test matrix to match declared Python version support (3.7–3.13).
The project declares support for Python 3.7–3.13 in setup.cfg (python_requires = >=3.7 and classifier entries), but the test matrix has been reduced to only 3.14 and pypy-3.11 in the main job. This removes testing for all currently-supported versions (3.7–3.13) while adding an untested future version (3.14). Only Python 3.7 is partially retained via an explicit include on ubuntu-22.04.
This change significantly reduces CI coverage and creates a mismatch between declared and tested compatibility. Restore testing for 3.8–3.13 in the main matrix, and clarify the intent behind 3.14 (beta testing, or removal of support for older versions?).
There was a problem hiding this comment.
The test matrix includes this snippet that extends it to cover Python 3.7.
include:
- os: 'ubuntu-22.04'
python-version: '3.7'
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
34-43: Test matrix still under-covers declared Python versions (3.7–3.14).The matrix now runs on
3.14,pypy-3.11, and a single included3.7job on ubuntu-22.04, butsetup.cfgdeclares support for CPython 3.7–3.14 (python_requires >=3.7plus classifiers). Dropping 3.8–3.13 from the matrix means most declared CPython versions are no longer exercised in CI.Either:
- Reintroduce at least a representative subset of CPython versions (e.g., 3.8 and 3.13 in addition to 3.7 and 3.14), or
- Narrow
python_requires/classifiers to match the actually tested range.This is essentially the same concern raised in the earlier review; the 3.7 include helps but doesn’t address missing 3.8–3.13 coverage.
🧹 Nitpick comments (2)
setup.cfg (1)
32-41: Python 3.14 classifier andreleaseextra look consistent; ensure release workflow uses the new extra.Adding the 3.14 classifier keeps
classifiersin sync with the CI goal of testing 3.14, and movingbuild/twineinto a dedicatedreleaseextra is a clean separation from generaldeveloptooling. Just make sure your release/publish workflows now install.[release](in addition to any other extras) so these constraints are actually applied during packaging.Also applies to: 79-94
.github/workflows/test.yml (1)
66-66: Optional: simplify the style-check condition now thatpypy-3.7is no longer in the matrix.
if: matrix.python-version != '3.7' && matrix.python-version != 'pypy-3.7'can be reduced to exclude just'3.7', sincepypy-3.7is not part of the currentpython-versionmatrix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/dependabot.yml(1 hunks).github/workflows/publish.yml(1 hunks).github/workflows/release-github.yml(1 hunks).github/workflows/test.yml(3 hunks)setup.cfg(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish.yml
🔇 Additional comments (3)
.github/dependabot.yml (1)
8-11: Dependabot pip schedule change is valid and aligned with the config format.The daily interval and structure under
updatesfor the pip ecosystem look correct; expect only increased PR frequency as a trade‑off..github/workflows/release-github.yml (1)
39-41: Release owner updated to match current GitHub org.Using
--user grafana-toolboxis consistent with the repository location (grafana-toolbox/grafana-client) and should resolve the stalepanodatareference, assuming the token has appropriate permissions..github/workflows/test.yml (1)
7-7: Trigger and install command adjustments look good.Broadening the path filters to
.github/workflows/**plus Python sources will ensure this workflow runs when CI config or code changes, and the updatedpip install --editable='.[test,develop]'quoting is safer for shells while keeping the intended extras install.Also applies to: 15-15, 63-63
Just maintenance.