Add new input uv-operations to Python test workflow - #66
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The python_version → python-version input rename is a breaking API change for existing @v2 consumers unless a backward-compatible alias/fallback is added.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the shared Python reusable workflows in platform-actions to make the test step configurable via a new input, and to standardize Python version input naming across workflows.
Changes:
- Add a new optional
uv-operationsinput totest.python.ymlto control whatuv run ...executes. - Rename
build.python.ymlinputpython_version→python-versionand pinactions/setup-pythonto a commit SHA.
Changed files:
.github/workflows/test.python.yml— What changed: adds a newworkflow_callinput and uses it in theuv runcommand. Validation result:⚠️ Warning- Issue:
uv-operationsis described as generic “UV operations” but is actually arguments touv run.
- Issue:
.github/workflows/build.python.yml— What changed: renames the Python version input and updatessetup-pythonusage. Validation result: ❌ Error- Issue: Renaming
python_versiontopython-versionis a breaking change for external@v2consumers; add a deprecated alias and makesetup-pythonaccept either input name.
- Issue: Renaming
Overall verdict: Issues found
File summaries
| File | Description |
|---|---|
| .github/workflows/test.python.yml | Adds configurable uv run arguments for Python test runs. |
| .github/workflows/build.python.yml | Standardizes Python version input naming and pins setup-python. |
Review details
Suppressed comments (2)
.github/workflows/build.python.yml:54
actions/setup-pythoncurrently reads onlyinputs.python-version, so callers using the legacypython_versioninput would still be ignored. Use a fallback expression so either input name works.
with:
python-version: ${{ inputs.python-version }}
.github/workflows/build.python.yml:10
- Renaming the workflow_call input from
python_versiontopython-versionis a breaking change for existing@v2consumers that still passpython_version. Keep the old input as a deprecated alias to preserve backward compatibility.
This issue also appears on line 53 of the same file.
python-version:
required: false
type: string
description: "What Python version to use."
default: "3.13"
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| workflow_call: | ||
| inputs: | ||
| python_version: | ||
| python-version: |
There was a problem hiding this comment.
Einig at denne er irriterande, men trur det er litt for risky. Vi lanserer endringane under @v2, så dette vil knekke pipelinen for dei som har spesifisert python_version.
Då må vi eventuelt ha ein overgong med både python-version og python_version under @v2
There was a problem hiding this comment.
Endte med å fjerne da det er lettere å ta det i ny major
| - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 | ||
| with: | ||
| python-version: ${{ inputs.python_version }} | ||
| python-version: ${{ inputs.python-version }} |
There was a problem hiding this comment.
| python-version: ${{ inputs.python-version }} | |
| python-version: ${{ inputs.python_version }} |
Hugs å reverter denne også
There was a problem hiding this comment.
🟡 Changes recommended
build.python.yml references an undefined input (inputs.python-version) even though the workflow declares python_version, which will break Python setup for callers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The functional changes are backward-compatible (optional input with default) and the action pinning is an improvement; only a minor PR-description mismatch was noted.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
| - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 | ||
| with: | ||
| python-version: ${{ inputs.python_version }} | ||
|
|
Jeg jobber med å oppgradere alternative-transport-realtime-processing til nye pipelines og ser behovet for å kunne spesifisere test-operasjoner. Vi har en del trege tester som vi kun kjører lokalt ved behov og bruker derfor
run: uv run --frozen pytest -m "not slow"når vi kjører testene i CI.I tillegg la jeg merke til at det er inkonsekvent bruk av python_version vs python-version i test vs build, så jeg retter opp i dette.