Skip to content

Conversation

@kieran-ryan
Copy link
Member

@kieran-ryan kieran-ryan commented Jul 26, 2025

🤔 What's changed?

  • Standardised Python installation, dependency management, project management, build and publishing to uv
    • Previously setuptools, build, twine, pypa/gh-action-pypi-publish, actions/setup-python
    • Migrated to dependency-groups standard (PEP-735)
    • Centralised dependencies to pyproject.toml (locked to uv.lock) rather than requirements.txt, tox.ini and pyproject.toml
  • Standardised Python linting to ruff - and enabled CI checks
    • Previously ruff, pyupgrade, flake8, flake8-bugbear and flake8-pyproject
    • Bump ruff to v0.12.5 from v0.8.6
    • Dropped tool.ruff.target-version configuration - with project.requires-python standard preferred (PEP-751)
    • Applied fixes and formatting (missed as not checked in CI)
  • Dropped legacy .egg-info metadata from distribution
  • SPDX licence expression over deprecated classifier (see guidance)
  • Test against built package within CI rather than source code

⚡️ What's your motivation?

  • Closes python: Test against build package #322 - running pytest with non-editable installs
  • Closes python: Use Poetry to build and release #40 - uv provides capabilities beyond poetry - we can rollout across the organisation
  • Improves contributing, onboarding, maintainability and robustness of Python implementation - such as aligning with modern Python standards, ensuring reproducible development environments, and ease of running locally similar to workflows - with the pipeline also streamlined
  • Eliminates unnecessary dependencies
  • Prevents publishing Python code that does not meet project linting criteria

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

♻️ Anything particular you want feedback on?

  • A future consideration would be to assess whether to run tox within the GitHub workflows for parity between local development and the pipeline. Retained pipeline (without tox) for now for preservation of cleanest workflow implementation that matches the assumed typical development run methods; though with an understanding to reassess in future, and as uv and tox develop further in this area. For awareness.

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

- Ruff now prefers standard 'project.requires-python' over 'ruff.target-version'
- fix type hint for internal `_media_type` function to account for `None` return
@kieran-ryan kieran-ryan self-assigned this Jul 26, 2025
@kieran-ryan kieran-ryan added the 🏦 debt Tech debt label Jul 26, 2025
@kieran-ryan kieran-ryan force-pushed the debt/refresh-py-tooling branch 2 times, most recently from e38e37e to 2eb03e3 Compare July 26, 2025 18:06
- include `pytest` in `dependency-groups` standard of `pyproject.toml`
- test, build and publish via `uv` in workflow
- remove install commands
- eliminate `twine` dependency
@kieran-ryan kieran-ryan force-pushed the debt/refresh-py-tooling branch from 6defef8 to b2bd681 Compare July 26, 2025 18:45
@kieran-ryan kieran-ryan force-pushed the debt/refresh-py-tooling branch from 6a1f469 to 5277022 Compare July 26, 2025 22:06
@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jul 26, 2025

uv provides capabilities beyond poetry - we can rollout across the organisation

Great to see you're picking this up!

Using tox in CI and updating makefile Python commands to use uv.

I'm not sure what you mean by using tox in CI.

I can help with updating the Makefile. Currently there are two commands to run the acceptance tests.

GHERKIN_GENERATE_EVENTS = python -m scripts.generate_events
GHERKIN_GENERATE_TOKENS = python -m scripts.generate_tokens

Make executes these with each file from the testdata/{good,bad} folders. For example:

python -m scripts.generate_tokens ../testdata/good/incomplete_feature_2.feature > acceptance/testdata/good/incomplete_feature_2.feature.tokens

How would I replicate this command with uv? Including any pre-requisitves like uv build. If you can write that down that should be enough info to update the make file.

Co-authored-by: M.P. Korstanje <mpkorstanje@users.noreply.github.com>
@kieran-ryan
Copy link
Member Author

kieran-ryan commented Jul 26, 2025

I'm not sure what you mean by using tox in CI.

There's a sweet spot to be worked out between tox, pre-commit and uv to eliminate dependency duplication (e.g. pytest and pre-commit are now referenced in pyproject.toml and tox.ini) and align what runs in the GitHub Actions workflows with the local development environment (e.g. running same commands locally as in our GitHub Actions, if we can) i.e. currently we don't run through tox in our GitHub Actions though have it as a configuration to run locally. As I've integrated our pre-commit into tox, potentially we could run the Python linting and Python testing through tox solely, and remove the pyproject.toml references... though on the other hand, you would probably want to install certain dependencies into a virtual environment through uv for the best IDE support for instance. Lots of tradeoffs... though tox and pre-commit can be made use uv under the head... so requires experimentation and may be something to revisit. Nice video reference I recall from Anthony Sottile on the trick he uses to run tox through a matrix of Python versions (see flake8 usage) - which is probably how we might do it: https://youtu.be/KKJL8bM4cis?si=RQfqEiwCok3qxkdM&t=615.

How would I replicate this command with uv? Including any pre-requisitves like uv build. If you can write that down that should be enough info to update the make file.

There's a number of different ways to run uv, so may also require experimentation.

Without any virtual environment created, running the below within the Python directory will create one, install an editable installation of the package along with its dependencies (afterwards, activate the virtual environment and run uv pip freeze to see them).

uv run scripts/generate_tokens.py

You can also run via the installed Python (installed via uv that is) and similar to the existing invocation. This may be redundant if the above run works fine.

uv run python -m scripts.generate_tokens

Within the GitHub Actions workflow, the uv setup action is activating the virtual environment before running the make acceptance to allow it to run... which is basically what was the expected workflow previously... you had to install it globally or in a virtual environment before running with make. Should we be able to get the above to work... it should hopefully be possible to run the make command without having to install those dependencies first.

I'm not sure how tricky this or how this will interact with the top-level Makefile... though hopefully the general idea makes sense in terms of standardising how we're running these commands... and preventing the need to install virtual environments or dependencies globally in order to do it.

Just an fyi as well that the uv build backend deprecates a legacy artefact in the package builds - compared to the older version of setuptools we are using - so I have a TODO to include that into a changelog entry.

@kieran-ryan kieran-ryan force-pushed the debt/refresh-py-tooling branch from 6264cf7 to 1a996cf Compare July 27, 2025 12:47
@kieran-ryan
Copy link
Member Author

eliminate dependency duplication (e.g. pytest and pre-commit are now referenced in pyproject.toml and tox.ini)

Pinned tox from v4.22 onwards - which provides support for the dependency groups standard - with test dependencies now centralised to pyproject.toml, rather than also being present in tox.ini.

- all commands now running through uv which manages dependencies with having to manage environments https://docs.astral.sh/uv/guides/scripts/
@kieran-ryan
Copy link
Member Author

Without any virtual environment created, running the below within the Python directory will create one, install an editable installation of the package along with its dependencies (afterwards, activate the virtual environment and run uv pip freeze to see them).

uv run scripts/generate_tokens.py

Integrated and working. Python acceptance test scripts now run without requiring to install the package and its dependencies; provided uv is installed.

@kieran-ryan
Copy link
Member Author

kieran-ryan commented Jul 27, 2025

@mpkorstanje I've resolved the outstanding items I mentioned to integrate uv into the makefile to also run the acceptance tests; and I've centralised development dependencies into pyproject.toml - with consideration in future to evaluate the tradeoffs of running tests and linting within GitHub workflows through tox - which is not used elsewhere across the organisation.

I've identified a constraint within the uv_build backend on the file permissions of two distribution metadata artifacts (see attached bug report issue with uv). Given the scope and the turnaround time within uv, I will await guidance or resolution on that issue; and otherwise change to another build system in the interim that supports the dependency groups standard (not the original setuptools); which would be a small change in the pyproject.toml. Passing context as I will be with limited laptop access over the coming weeks, should a uv_build backend be released with a fix meanwhile.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Cheers. Please flag this for review when everything is ready.

Bump uv to v0.8.4 which corrects permissions on build metadata files to remove write and executable permissions and provide read permissions astral-sh/uv#14920
@kieran-ryan kieran-ryan force-pushed the debt/refresh-py-tooling branch from 8b8bde9 to 49d8c3a Compare August 6, 2025 08:15
@kieran-ryan kieran-ryan marked this pull request as ready for review August 6, 2025 08:20
@kieran-ryan
Copy link
Member Author

@mpkorstanje ready for review with resolution of our observed issue now released with uv v0.8.4.

@mpkorstanje
Copy link
Contributor

Cheers! I'm short on time this week. But should get around to it on the next.

@blaisep
Copy link
Member

blaisep commented Aug 6, 2025

@kieran-ryan this all looks soooo nice, thank you.

@blaisep
Copy link
Member

blaisep commented Aug 6, 2025

@kieran-ryan I had a thought about the whole pyproject.toml vs tox.ini dependency question. Perhaps we should think about it differently.
Inspired by Brian Okken's comment that he uses a single system-wide pytest, I have noticed that tools like pytest, pipx, ruff, uv, etc. get used by projects (typically in --dev) however they are not consumed by those projects. In a sense, the unit of revision is different.
So, if the tools were available in the system image, along with the python interpreter, git client, etc. the project would be just fine.
Having a builder image that we can refer to simplifies dependency management and reduces the number of times we have to download these tools into the environment.
In fact, the Chainguard folks over at Wolfi already provide an image with pytest included (see config )

cc: @mpkorstanje

@blaisep
Copy link
Member

blaisep commented Aug 7, 2025

@kieran-ryan , TIL the github ubuntu runners come with Ansible pre-installed that means that we could run an Ansible playbook right out the gate without waiting for apt to do anything, and Ansible is wicked fast because it's smart about running tasks in parallel.
I'll also look into how to get ARA to capture the results from the github action runner because that is a lot easier to peruse than the console logs.

@dmsimard
Copy link

dmsimard commented Aug 7, 2025

Hi, blaisep summoned me :p

@kieran-ryan , TIL the github ubuntu runners come with Ansible pre-installed that means that we could run an Ansible playbook right out the gate without waiting for apt to do anything, and Ansible is wicked fast because it's smart about running tasks in parallel. I'll also look into how to get ARA to capture the results from the github action runner because that is a lot easier to peruse than the console logs.

As long as ara is installed alongside wherever ansible-core is installed (which seems to be globally via pipx?) it should work to record playbooks.

The main question is how you make the reporting data available. By default it records to a local sqlite database and it's possible to generate a static html report (ara-manage generate) but then you need to find out where to host that and make it available.

If you can host an instance of ara somewhere, you could send the data from a github action a bit like this: https://github.com/ansible-community/antsibull-build/pull/339/files

For ara's own CI jobs we send them to our live demo with labels after commit SHAs and pull requests, for example: https://demo.recordsansible.org/?label=ref:refs/pull/610/head

👋

@blaisep
Copy link
Member

blaisep commented Aug 7, 2025

Thank you , I will study this carefully https://gist.github.com/blaisep/1d4b954ba282a11d511f0a708689aa0a

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Aug 7, 2025

I haven't gotten around to looking at this yet - just catching up on comments while on the train.

@blaisep

Inspired by Brian Okken's comment that he uses a single system-wide pytest, I have noticed that tools like pytest, pipx, ruff, uv, etc. get used by projects (typically in --dev) however they are not consumed by those projects. In a sense, the unit of revision is different.
So, if the tools were available in the system image, along with the python interpreter, git client, etc. the project would be just fine.
Having a builder image that we can refer to simplifies dependency management and reduces the number of times we have to download these tools into the environment.

The most important aspect to optimize for is the human. So the workflow we should be looking for should is whatever is the most common denominator for a given language. No new skills required. No new tools required.

Ideally someone familiar with <language> and <build-tool> installed clones this repository then does:

cd <language>
<build-tool> test

And to deploy follows that up with:

<build-tool> deploy

And then everything just works.

Then to ensure that this workflow stays working we do the exact same thing in CI. Externalizing build tools breaks this guarantee. Which means that eventually the build breaks for the human too - silently.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Aug 7, 2025

Inspired by Brian Okken's comment that he uses a single system-wide pytest, I have noticed that tools like pytest, pipx, ruff, uv, etc. get used by projects (typically in --dev) however they are not consumed by those projects. In a sense, the unit of revision is different.

On closer inspection, I'm also not convinced that this line of reasoning is correct. While dev dependencies are not transient dependencies of the artifact produced by the build process, they are very much dependencies of the build process.

But this is going somewhat off-topic.

@mpkorstanje mpkorstanje merged commit 50cbc56 into main Aug 12, 2025
37 checks passed
@mpkorstanje mpkorstanje deleted the debt/refresh-py-tooling branch August 12, 2025 09:06
kieran-ryan added a commit that referenced this pull request Aug 16, 2025
Introduced on #439 merge conflict resolution from #451 changes to bump the checkout action while the other dropped the action ‘name’ and ‘uses’ the action directly, so the merged change as before had no dash and caused a syntax error
mpkorstanje pushed a commit that referenced this pull request Aug 16, 2025
Introduced on #439 merge conflict resolution from #451 changes to bump the checkout action while the other dropped the action ‘name’ and ‘uses’ the action directly, so the merged change as before had no dash and caused a syntax error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏦 debt Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

python: Test against build package python: Use Poetry to build and release

4 participants