feat: modernize Python tooling (pyproject.toml + uv + semantic-release) - #242
Open
irfanuddinahmad wants to merge 36 commits into
Open
irfanuddinahmad wants to merge 36 commits into
irfanuddinahmad wants to merge 36 commits into
Conversation
openedx-webhooks
added
open-source-contribution
|
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. |
|
Hi @irfanuddinahmad! Would you mind taking a look at the branch conflicts that have popped up? Thanks! |
15 tasks
irfanuddinahmad
force-pushed
the
irfanuddinahmad/uv-migration
branch
from
July 15, 2026 04:10
b865b39 to
aaba87f
Compare
farhan
self-requested a review
July 16, 2026 10:42
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-runner/uv-venv-lock-runner, update Makefile targets, and fix .readthedocs.yaml to install docs deps via uv. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace the manual GitHub-release-triggered publish workflow with python-semantic-release: pushes to main with conventional commits now automatically bump the version, tag it, and publish to PyPI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… MANIFEST.in entries
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.
packages.find.exclude only stops setuptools from registering the tests
subpackage; with include-package-data=true it still swept tests/*.py into
the wheel as package_data. Verified: built wheels before/after this fix --
tests/__init__.py and tests/test_views.py were present in the wheel prior
to this commit and are absent after, while test_utils/ (the intentionally
shipped factories module) is unaffected.
Also removes MANIFEST.in references to requirements/base.in and
requirements/constraints.txt, which no longer exist after the uv migration.
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. Also adds venv/ and .venv/ to .gitignore (previously absent). 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>
@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 openedx_ledger/ to src/openedx_ledger/, 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 the quality env's pylint/pycodestyle/isort targets - Makefile: prefix src/openedx_ledger onto the standalone isort/style/lint targets and the 5 localization targets that cd into the package; extract_translations/compile_translations' relative ../manage.py climbs 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/openedx_ledger - MANIFEST.in: recursive-include path updated - Dockerfile needs no change: it COPYs the whole repo rather than naming the package directory directly Verified: uv build --wheel + twine check pass (direct proof the where=["src"] packaging change works). Could not run the full pytest/quality/docs tox matrix locally -- this machine has no libmysqlclient/pkg-config to build the mysqlclient C extension (a [project.dependencies] entry, so it's required for every uv sync regardless of group), same environment limitation hit previously on enterprise-access. Relying on CI for full-matrix confirmation.
irfanuddinahmad
force-pushed
the
irfanuddinahmad/uv-migration
branch
from
July 17, 2026 10:15
a01e192 to
3e3ac11
Compare
|
tests are failing |
CI failed after the src-layout move: pytest raised ModuleNotFoundError for openedx_ledger.tests when collecting src/openedx_ledger/tests/test_views.py. Root cause: these two envs were the only ones in this repo still using tox-uv's non-lock uv-venv-runner, which builds and installs a real (non-editable) sdist for testing. That build genuinely excludes openedx_ledger.tests per packages.find(exclude=["*tests"]) -- correct for the published PyPI wheel, wrong for a test run. In flat layout this was masked because the sdist-installed copy in site-packages was shadowed on sys.path by the physically-identical openedx_ledger/ directory at repo root; moving the package under src/ removes that accidental shadow (the exact masking effect the src-layout/flat-layout packaging guide warns about), so the real gap surfaced. docs/quality already used uv-venv-lock-runner (uv sync, editable install) and were unaffected. Verified the fix mechanism in an isolated repro (same packages.find exclude pattern, in-package tests subpackage, tox + tox-uv): switching uv-venv-runner -> uv-venv-lock-runner changes the install from a built sdist to `uv sync`'s editable install, and the ModuleNotFoundError goes away. Could not run the real repo's tox matrix locally (mysqlclient build environment limitation) -- pushing to confirm via CI.
farhan
suggested changes
Jul 20, 2026
| "edx", | ||
| ] | ||
|
|
||
| dynamic = ["readme", "version"] |
There was a problem hiding this comment.
Making readme dynamic is non-standard for this migration pattern. Target state is dynamic = ["version"] only. The readme field can be set statically under [project] or omitted — the distribution still builds correctly.
| include-package-data = true | ||
|
|
||
| [tool.setuptools.dynamic] | ||
| readme = {file = ["README.rst", "CHANGELOG.rst"], content-type = "text/x-rst"} |
There was a problem hiding this comment.
CHANGELOG.rst must be deleted for PyPI repos that use python-semantic-release. Drop it from this list:
readme = {file = ["README.rst"], content-type = "text/x-rst"}Also delete the CHANGELOG.rst file from the repo root.
Contributor
Author
There was a problem hiding this comment.
Reversed in b2eff5b -- CHANGELOG.rst has been restored, per a later decision to keep it as a plain static historical record even with automated release notes/GitHub Releases as the primary mechanism going forward. Automation stays off (changelog: "false" in release.yml, no insertion marker, no semantic-release changelog wiring), so this is just the file's history, not a re-enabling of anything.
Addresses farhan's review on #242: - ci.yml: add enable-cache/python-version to astral-sh/setup-uv and drop the now-redundant actions/setup-python step; SHA-pin codecov-action (was floating on @v7, inconsistent with every other repo in this batch) - pyproject.toml: make readme static (dynamic = ["version"] only -- dynamic readme is non-standard for this migration pattern), add [tool.uv] package = true, migrate .coveragerc into [tool.coverage.*] (also fixing its stale source=edx_ledger -> source_pkgs=["openedx_ledger"]) and delete the old file - Drop CHANGELOG.rst from the dynamic readme file list (before removing the dynamic readme entirely) and delete it -- python-semantic-release + GitHub Releases is the changelog of record now. Also removes the now-stale MANIFEST.in include and the docs/changelog.rst page (and toctree entry) that only existed to embed it into Sphinx docs. - Makefile: requirements target now also installs tox as a uv tool (uv tool install tox --with tox-uv), so tox is available on a fresh checkout that only has uv installed Verified: pyproject.toml parses, uv build --wheel + twine check pass (direct proof the readme/coverage config changes work). Could not run the full tox matrix locally -- same mysqlclient build environment limitation as before (no libmysqlclient/pkg-config on this machine).
farhan
suggested changes
Jul 28, 2026
added 3 commits
July 28, 2026 15:35
`uv tool install tox --with tox-uv` was added to the `requirements` target in response to review feedback, but it installs an unpinned, un-lockfiled tox outside uv.lock. Every other tox invocation in this repo (test-all/quality/pii_check Makefile targets, ci.yml) uses `uv run tox` via the ci/dev dependency-groups, which already declare tox/tox-uv. Remove the added line to keep this consistent.
…olutions [project].dependencies had an unconstrained "Django" entry, so uv.lock resolved a single Django version (5.2.x) for the whole project. The django42 tox env then force-overrode just the Django package afterward via tox's deps=, while every other locked/transitive dependency stayed resolved against the Django-5.2 graph -- not a real, independent resolution for the 4.2 case. Add a django42 dependency-group pinning Django>=4.2,<4.3, pin the default test group to Django>=5.2,<6.0, and declare the two as conflicting via [tool.uv].conflicts so uv locks a genuine fork for each. Update tox.ini's testenv/pii_check envs to select the matching group per Django factor instead of overriding just the Django package. `uv lock` confirms this was a real bug: django-filter also forks to 25.1 (django42) vs 25.2 (django52) -- the single prior resolution had been silently testing 4.2 against a django-filter version potentially never resolved against Django 4.2's constraints. Verified locally (mysqlclient excluded due to a sandbox limitation building it; unrelated to this change): `uv sync --group django42` and `--group test` each install the correct, independently-resolved Django version, `manage.py check` and the full pytest suite (34 passed) pass under both, and quality/pii_check pass under django42.
…gration # Conflicts: # CHANGELOG.rst # requirements/base.txt # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/quality.txt # requirements/test.txt # src/openedx_ledger/__init__.py
added 3 commits
July 28, 2026 16:06
The merge of origin/main into this branch auto-merged uv.lock as plain text (it wasn't flagged as conflicted), which left the auto-derived [tool.uv]-conflicts expansion for the django42 group with its "doc" and "quality" entries swapped relative to what a fresh `uv lock` produces. uv 0.11.33 (what CI's setup-uv currently installs) treats this as a stale lockfile and fails `--locked`/`--check`; the slightly older uv 0.11.30 tolerated it, which is why this wasn't caught before pushing. Verified: `uv lock --check` and `uv sync --locked --group <g>` pass clean uv 0.11.33 for django42/test/quality/dev/ci; pytest (34 passed), manage.py check, pylint/pycodestyle/isort, and pii_check all pass under both django42 and django52.
The previous push's lockfile fix regenerated uv.lock with system uv 0.11.33, but the "uv" PyPI package itself (a transitive dependency of tox-uv-bare, used internally by tox-uv's uv-venv-lock-runner inside each tox env) was still locked at the older 0.11.28. These two uv releases disagree on the canonical serialized order of the auto-derived [tool.uv].conflicts entries, so whichever version last wrote the lock, the other considered it stale under --locked -- CI's "Install Dependencies" step (system uv, "latest" via setup-uv) and its "Run Tests" step (tox-uv's pinned "uv" package) were fighting each other. `uv lock --upgrade-package uv` bumps the locked uv package to 0.11.33, matching today's system uv, so both steps agree. Verified: `uv sync --locked --group <g>` passes clean under uv 0.11.33 (matching CI) for django42/test/quality/doc/dev/ci; pytest (34 passed) and quality checks pass under both django42 and django52.
Unlike [testenv] (whose generated env names py312-django42/py312-django52
genuinely contain those factors), [testenv:pii_check] is a fixed-name
env ("pii_check"), so a factor-conditional dependency_groups (as used
for [testenv]) never matches and left pii_check with zero groups
installed -- confirmed by CI: "Exception running subprocess [Errno 2]
No such file or directory: 'code_annotations'".
pii_check was never actually parametrized by Django version (its old
deps= factor overrides were dead code for the same reason), so restore
its original always-on `test` group, matching quality/docs' pattern of
an unconditional dependency_groups for fixed-name envs.
Verified via `tox config -e <env>` that django42/django52/pii_check/
quality/docs all resolve to the intended group, and that
code_annotations (pii_check) now installs and runs successfully.
10 tasks
added 7 commits
July 30, 2026 14:47
…eneration A previous migration pass deleted CHANGELOG.rst and set changelog: "false" in release.yml's python-semantic-release step. There is no ticket requirement to disable changelog generation, and deleting the file discarded real historical release notes. Restore CHANGELOG.rst from the commit before its deletion, add the insertion marker PSR's "update" mode looks for, remove the changelog: "false" override, and configure [tool.semantic_release.changelog] to update the existing RST file in place.
…mplate python-semantic-release now handles both version bumping and changelog generation automatically on release, so these manual checklist items are obsolete.
python-semantic-release's release.yml now creates the tag and publishes to PyPI automatically on merge -- these were no longer real manual steps.
"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.
# Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/constraints.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/pip.txt # requirements/quality.txt # requirements/test.txt
- .github/workflows/ci.yml: kept our astral-sh/setup-uv, adopted main's actions/checkout bump to v7.0.1. Same conflict pattern as the other repos in this migration effort. - .github/workflows/pypi-publish.yml: modify/delete -- kept deleted, superseded by this migration's release.yml. Main's change was only a routine actions/checkout version bump on that now-dead file. No uv pip usage found in this repo. The django42/django52 fork-resolution fix was already correct and complete here (verified: uv lock resolves two genuinely different graphs, 4.2.30 vs 5.2.16 with django-filter also diverging) -- no changes needed.
Set changelog: "false" on the PSR release step and remove the [tool.semantic_release.changelog] config / insertion marker. Checked against openedx/XBlock's actual production release.yml (the one repo in this effort that has cut real automated releases) -- every run passes changelog: false and invokes `semantic-release -v version --no-changelog`, and the repo has zero github-actions[bot] commits ever. The auto-changelog config this migration previously added was only ever verified via a local dry-run prototype, never against a real release. feanil flagged the same issue on openedx/DoneXBlock#388: "I thought we were not going to add changelogs since they can't be updated by python-semantic-release the way we have it setup."
added 5 commits
August 20, 2026 15:44
Resolved: kept our deletion of pypi-publish.yml (superseded by release.yml) and our uv-based ci.yml over master's trivial setup-python SHA bump.
pip install -e + pip freeze + source venv/bin/activate predates the uv migration -- make requirements (uv sync) now installs this repo into its own .venv already, and there's no venv/ directory to activate.
- pypa/gh-action-pypi-publish: revert the hash-pinned SHA back to the stable @release/v1 tag, matching the rest of this effort's PRs. A hash-pinned version broke PyPI publishing previously for this org. - Makefile docs/quality/pii_check targets: inline the actual tox.ini commands via uv run/uv sync instead of shelling out to `uv run tox -e <env>`, matching the no-tox-in-Makefile convention already used elsewhere (edx-enterprise, enterprise-access). tox.ini itself is untouched; test-all still uses bare `uv run tox` for the actual Python/Django matrix run, which is what tox is for.
…gration # Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/constraints.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/pip.txt # requirements/quality.txt # requirements/test.txt
The just-merged pylintrc (from main) was regenerated by edx-lint 6.2.0 and adds the new pii-invalid-no-pii-annotation check's [PII] pii-terms option. Our lock was still on edx-lint 6.1.0, whose pylint plugin doesn't recognize that option, breaking `make quality` with E0015: Unrecognized option found: pii-terms.
Contributor
Author
|
Rebased on master — conflicts resolved, CI green now. |
Automation is disabled (changelog: false) and the file was already told to farhan as removed; a later commit re-enabled/disabled automation without re-deleting it. Deleting for real this time, matching the batch convention.
This repo has immutable releases enabled, which freezes a release's assets the moment it's published. The old flow (main PSR step publishes the release, a separate publish-action step attaches assets afterward) can never work under that constraint -- it would 422 on the first real release. Matches the fix already proven and merged on openedx/sample-plugin#57 and validated end-to-end on openedx/event-tracking#434: build without publishing (vcs_release: false), then create the release with dist/* attached in one gh release create call.
…e-plugin actions/checkout, python-semantic-release, actions/upload-artifact and actions/download-artifact were pinned to outdated commit SHAs, and pypa/gh-action-pypi-publish was pinned to the floating `release/v1` tag rather than a commit SHA at all (a supply-chain risk, since that tag can be moved). Bring all five in line with the versions already verified against openedx/sample-plugin's release.yml (v7.0.1, v10.6.2, v7.0.1, v8.0.1, and v1.14.2 respectively), each cross-checked via `gh api .../commits/<sha>` and the corresponding tag ref. This repo's custom draft-release step (to work around immutable-releases freezing assets) and its branch-conditional guard were already correct and are left as-is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reverses 1a91f62 per explicit user decision -- keeping the historical changelog intact as a plain static file (no insertion marker, no semantic-release wiring; automation stays disabled via changelog: "false" in release.yml, unchanged). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
9a6b160 removed the hardcoded __version__ = "1.8.0" (correctly, since version is now dynamic via setuptools-scm) but never replaced it with the importlib.metadata equivalent, unlike every sibling repo in this migration batch. openedx_ledger.__version__ has been silently missing since. Restored using the same pattern used elsewhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
farhan
reviewed
Sep 15, 2026
| ********** | ||
| * Nothing unreleased | ||
|
|
||
| [2.0.0] |
| @@ -15,11 +15,6 @@ Unreleased | |||
| ********** | |||
| * Nothing unreleased | |||
There was a problem hiding this comment.
We should deprecate this file as we have moved to PSR
| @@ -1,5 +1,7 @@ | |||
| [tox] | |||
| envlist = py{312}-django{42, 52} | |||
There was a problem hiding this comment.
Please add docs and quality environments
farhan
suggested changes
Sep 15, 2026
| uses: python-semantic-release/python-semantic-release@9a026e9303981c866c3425723009becb2437c757 # v10.6.2 | ||
| with: | ||
| github_token: ${{ secrets.OPENEDX_SEMANTIC_RELEASE_GITHUB_TOKEN }} | ||
| git_committer_name: "github-actions" |
There was a problem hiding this comment.
OPENEDX_SEMANTIC_RELEASE_GITHUB_TOKEN is not in the pre-merge checklist. If this org-level secret is not configured for this repo, the release job will fail silently on the first push to main.
| python manage.py check | ||
| pytest {posargs} | ||
|
|
||
| [testenv:docs] |
There was a problem hiding this comment.
We should either define docs commands in the make file or keep them here
but shouldn't define them at multiple places
|
|
||
| docs: ## generate Sphinx HTML documentation, including API docs | ||
| tox -e docs | ||
| uv sync --group doc |
There was a problem hiding this comment.
https://github.com/openedx/openedx-ledger/pull/242/changes#r4016431241
referring the relevant comment
| id: release | ||
| uses: python-semantic-release/python-semantic-release@9a026e9303981c866c3425723009becb2437c757 # v10.6.2 | ||
| with: | ||
| github_token: ${{ secrets.OPENEDX_SEMANTIC_RELEASE_GITHUB_TOKEN }} |
There was a problem hiding this comment.
We should prefer to use GITHUB_TOKEN
https://github.com/openedx/sample-plugin/blob/main/.github/workflows/release.yml#L53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 tomainwith conventional commits now automatically bump the version, tag it, and publish to PyPI (reusing the existingPYPI_UPLOAD_TOKENsecret).readthedocs.yaml, which pointed at the now-deletedrequirements/doc.txtPart of openedx/public-engineering#513.
Test plan
uv lockresolves cleanlyuv build --wheel(withSETUPTOOLS_SCM_PRETEND_VERSION) succeeds, producing a correctly versioned wheeluv sync/uv run toxon my machine (nolibmysqlclient/pkg-configlocally to buildmysqlclientfrom source) -- CI's ubuntu-latest runners have these preinstalled, so this needs verifying there.Fixed post-review
openedx_ledger.__version__was silently dropped during thepyproject.tomlconsolidation commit -- the hardcoded
__version__ = "1.8.0"was correctly removed(version is now dynamic via setuptools-scm) but never replaced with the
importlib.metadataequivalent, unlike every sibling repo in this migration batch.Restored; verified
import openedx_ledger; openedx_ledger.__version__resolves to2.0.0(matching the current git tag) and the full test suite (34 tests) passes.Release readiness (pre-merge blocker)
openedx-ledger→ GitHub repo
openedx/openedx-ledger, workflowrelease.ymlDo not merge until both boxes are checked -- until then,
publish_to_pypiwill failon first merge to
main(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