feat: modernize to uv + pyproject.toml + semantic-release - #190
Open
irfanuddinahmad wants to merge 26 commits into
Open
irfanuddinahmad wants to merge 26 commits into
irfanuddinahmad wants to merge 26 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #190 +/- ##
==========================================
- Coverage 97.78% 94.36% -3.43%
==========================================
Files 4 3 -1
Lines 226 71 -155
Branches 8 6 -2
==========================================
- Hits 221 67 -154
+ Misses 5 4 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
10 tasks
mphilbrick211
added
the
needs reviewer assigned
Replace setup.py with PEP 621 static metadata in pyproject.toml. setuptools-scm now derives the version from git tags instead of the hardcoded __version__ in ccx_keys/__init__.py; that attribute is kept for backward compatibility but now reads the installed package version via importlib.metadata. Coverage configuration is consolidated from .coveragerc into [tool.coverage.*]. Part of openedx/public-engineering#506.
Replace requirements/*.in/*.txt with PEP 735 dependency groups in pyproject.toml, resolved into a committed uv.lock. Update tox.ini to use tox-uv with the uv-venv-lock-runner, and update the Makefile and CI workflow to install dependencies via uv sync instead of pip-compile / pip-sync. CI now uses astral-sh/setup-uv (SHA-pinned) with caching, runs each tox env via `uv run tox -e <env>`, and checks out with fetch-depth: 0 so setuptools-scm can see tags during test runs. Part of openedx/public-engineering#506.
Add python-semantic-release configuration to pyproject.toml (major_on_zero = false, allow_zero_version = true) and a release.yml workflow that runs the CI suite, cuts a release with python-semantic-release on pushes to master, and publishes to PyPI via OIDC trusted publishing (no stored API token). Replace the old tag-triggered, token-based pypi-publish.yml, which would otherwise race with the new release.yml on the first semantic-release tag. ci.yml gains a workflow_call trigger and drops its push trigger, since release.yml now owns the master-branch path. commitlint.yml already existed and needed no changes. The latest git tag (2.0.2) matches the actual latest version published on PyPI, so the pre-flight check for enabling semantic-release passes. Part of openedx/public-engineering#506.
The importlib.metadata fallback added in the pyproject.toml migration (except PackageNotFoundError: pass) can't be exercised in CI since the package is always installed there, so it showed up as uncovered patch diff and tripped codecov/patch. Mark it pragma: no cover, consistent with the exclude_lines already configured in [tool.coverage.report].
The pyproject.toml coverage config added in this migration omits ccx_keys/tests/* from measurement, per this effort's standard [tool.coverage.run] omit pattern (a test file trivially covers itself just by running, so counting it was inflating the reported package coverage). That's a one-time, ~3-4% drop in the reported baseline, not an actual reduction in production-code coverage (ccx_keys/key.py and ccx_keys/__init__.py are both 100%, ccx_keys/locator.py is unchanged at 95%). Add a threshold so codecov/project reflects that instead of flagging every PR against a baseline that no longer measures the same thing. Sibling repos already migrated in this same effort (XBlock, openedx-events, edx-enterprise) all carry an equivalent codecov.yml.
Move ccx_keys/ to src/ccx_keys/ per the confirmed decision on openedx/public-engineering#506 (2026-07-15): src/ layout is in scope for this modernization cycle, with precedent set in xblock-core and xblock-extras. A flat layout's editable install can resolve inconsistently for tools like mypy; src/ layout avoids that ambiguity. Update [tool.setuptools.packages.find] (where = ["src"]), [tool.coverage.run].source, tox.ini's test/pylint/pycodestyle target paths, and MANIFEST.in's recursive-include path accordingly. Also drop MANIFEST.in's two remaining requirements/base.in and requirements/constraints.txt lines, both dead references to files requirements/ deleted in the dependency-management commit. Verified locally: `uv sync` installs the package in editable mode and `python -c "import ccx_keys"` resolves to src/ccx_keys/__init__.py; `uv run tox` (both unittest and quality envs) passes unchanged.
Verified every uses:@sha in release.yml against the GitHub API (repos/<owner>/<repo>/commits/<sha>). While these particular SHAs happened to be valid commits (unlike the same pins in sibling PRs opaque-keys/openedx-core, where they were swapped/fabricated), pinning these 4 actions isn't required by the migration ticket and isn't done by openedx/XBlock's actual, already-releasing release.yml. Reverted python-semantic-release, publish-action, upload-artifact, and download-artifact to plain version tags matching XBlock exactly, to avoid this whole class of error going forward. pypa/gh-action-pypi-publish was already correctly pinned to the real commit SHA (not the tag object SHA some sibling PRs used) -- left unchanged.
…ration
This repo never had a CHANGELOG.rst. Per the modernize-python-tooling
effort's decision, repos should not disable changelog generation when
using python-semantic-release -- there's no reason to throw away
release-to-release history.
- Add CHANGELOG.rst containing only the insertion marker; PSR's
"update" mode will build all version sections above it going forward.
- Remove changelog: "false" from release.yml's PSR step.
- Configure [tool.semantic_release.changelog] to update CHANGELOG.rst
in place using the insertion marker, RST output.
- Set tag_format = "{version}" to match this repo's actual bare
X.Y.Z tag convention (confirmed via git tag --sort=-v:refname);
without this PSR would not recognize any prior release tag.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same reasoning as taxonomy-connector#316/edx-enterprise-data#693/ opaque-keys#461 in this same lessons-learned pass: 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 only ever affects this PR's own diff -- once merged, the new percentage becomes the baseline for every future comparison. A permanent threshold had no real job to do. No uv pip usage and no Django-version matrix in this repo (pure-Python key-parsing library, no framework dependency at all) -- nothing else to check against the recurring bugs found in the enterprise-track batch.
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."
Reviewers on this effort (farhan, feanil) have repeatedly asked to delete multi-line AI-written justification comments from committed files. Moved the detail to this commit message instead: This tooling migration correctly omits ccx_keys/tests/test_ccx_keys.py from [tool.coverage.run] instead of counting the test file's own statements toward coverage -- a one-time ~3-4% discontinuity in this PR's base-vs-head comparison, not a real regression. target: auto means the new percentage becomes the baseline once this merges; no threshold was added.
CI switched to codecov/codecov-action; the Coveralls badge/link pointed at a service this repo no longer reports to. Also fixed the build-status badge referencing ci.yaml (the actual workflow is ci.yml) and pointing at the wrong repo's Actions page.
Inline tox.ini's [testenv:quality] commands via uv run/uv sync instead of shelling out to `uv run tox -e quality`, matching the no-tox-in- Makefile convention already used elsewhere. tox.ini itself is untouched; CI's own matrix testing still uses tox directly.
The 5% threshold described in this PR's own description (to absorb the one-time coverage-measurement shift from omitting tests/* -- see that section) was never actually added to the file. Without it, target: auto requires zero drop from the auto-detected base, which is exactly what codecov/project has been failing on.
This reverts commit ec6354d.
irfanuddinahmad
force-pushed
the
irfanuddinahmad/modernize-python-tooling
branch
from
August 27, 2026 08:18
d9dcfdf to
8cad4c7
Compare
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.
A prior commit (b2d0b56) deliberately un-pinned python-semantic-release, actions/upload-artifact, and actions/download-artifact to plain version tags, reasoning that SHA-pinning wasn't required by the migration ticket and that openedx/XBlock's release.yml doesn't do it either. openedx/sample-plugin is this org's designated gold-standard reference for release.yml, and it does pin all of these (each SHA independently re-verified against the action's own tag history via the GitHub API). Restored the pins to match, and picked up the version bumps that came with it: python-semantic-release v10.6.1 -> v10.6.2, and pypa/gh-action-pypi-publish v1.14.1 -> v1.14.2. Also brought release.yml in line with sample-plugin in three more ways: - added `if-no-files-found: error` to the dist upload step, so a release that produced no artifacts fails loudly instead of silently. - added the `github.ref_name == 'master'` guard to publish_to_pypi's `if`, matching the release job's own guard, so the job can't fire on a non-default-branch workflow run. - dropped `fetch-depth: 0` from the release job's checkout; python-semantic-release auto-deepens a shallow clone itself before evaluating version history, so it's never needed there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ccx_keys/__init__.py sets __version__ from importlib.metadata; nothing else in the package reads or parses __version__, so a shallow clone is sufficient for CI. sample-plugin's own backend-ci.yml doesn't set fetch-depth: 0 either. Matches that reference. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The comment pointed at "see PR description" for an explanation, but the description's account is stale (references a 5% threshold that a later commit removed). Coverage config itself stays out of scope for this migration -- just dropping the stale comment, no change to target/threshold behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
salman2013
suggested changes
Sep 16, 2026
| run_tests: | ||
| name: Tests | ||
| runs-on: ${{ matrix.os }} | ||
| name: ${{ matrix.toxenv }} |
There was a problem hiding this comment.
should we follow this here?
openedx/staff-graded-xblock#394 (comment)
Author
There was a problem hiding this comment.
Good catch, thanks for the staff-graded-xblock#394 link — confirmed the same risk here. Reverted to a static job name in 6950ea3.
| @@ -0,0 +1,10 @@ | |||
| coverage: | |||
This file's original justification (a threshold buffer for the one-time coverage-measurement drop from the migration's new [tool.coverage.run] omit patterns) was already reverted in a prior commit. What's left (target: auto, patch: 90%, comment: false) is generic config the migration doesn't need -- codecov's own zero-config defaults are sufficient, matching every other non-justified repo in this batch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
${{ matrix.toxenv }} produces bare check names (django42, quality, ...)
instead of the pre-migration "Tests (ubuntu-latest, 3.12, <toxenv>)".
If branch protection requires the old check names, they'd show as
permanently pending after this merges -- same required-status-check
breakage already confirmed on enterprise-access#1015 and flagged on
staff-graded-xblock#394.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
salman2013
approved these changes
Sep 16, 2026
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
Modernize
ccx-keysto uv + pyproject.toml (PEP 621/735) + python-semantic-release.Part of openedx/public-engineering#506.
setup.pywithpyproject.toml(PEP 621 static metadata);.coveragercconfig folded into[tool.coverage.*]uvwith PEP 735 dependency groups (test-base,test,quality,ci,dev); commituv.locktox.inito usetox-uvwithuv-venv-lock-runnerci.yml) to useastral-sh/setup-uv, SHA-pin all actions, addfetch-depth: 0so setuptools-scm sees tagspython-semantic-release+release.yml, replacing the old tag-triggeredpypi-publish.yml; publish to PyPI via OIDC trusted publishing (no stored token)commitlint.ymlalready existed and needed no changesccx_keys/tosrc/ccx_keys/— per the confirmed decision on public-engineering#506 (2026-07-15) thatsrc/layout is in scope this cycle, precedented byxblock-core/xblock-extrasRemoved
Deleted files:
setup.py,.coveragerc,requirements/(all.in/.txtfiles),.github/workflows/pypi-publish.ymlRemoved Makefile targets:
$(COMMON_CONSTRAINTS_TXT)wget target (no longer needed —edx_lint write_uv_constraintshandles constraints now)Not included
No ruff/lint-tooling changes — pylint/pycodestyle/edx-lint are left exactly as configured before; this migration is tooling-only, not a linting overhaul.
Versioning
Dynamic —
setuptools-scmderives the package version from git tags (version_scheme = "only-version",local_scheme = "no-local-version",fallback_version = "0.0.0"). Confirmed the latest git tag (2.0.2) matches the latest version actually published on PyPI, so the first automated release frompython-semantic-releaseshould compute a version strictly newer than 2.0.2.Testing Notes
Verified locally:
uv lockresolves cleanlyuv sync --group devsucceedsuv run tox— bothunittest(79 tests) andquality(pycodestyle + pylint) passpython -m build(the exactbuild_commandsemantic-release will invoke) produces a correct wheel: entry points, SPDX license expression, and README-as-long-description all verified by inspecting the built wheel's METADATA; test files are correctly excluded from the wheelCould not verify locally (needs CI / a maintainer):
secrets.CODECOV_TOKENavailability)edx-ccx-keys, as far as I can tell — this needs a maintainer with PyPI project-owner access to addopenedx/ccx-keys'srelease.yml/publish_to_pypienvironment as a trusted publisher on pypi.org. This is not a blocker for merging this PR, but the first automated release will fail at thepublish_to_pypijob until it's set up. (OPENEDX_SEMANTIC_RELEASE_GITHUB_TOKENitself is already a standard org-level secret used by many other repos.)codecov.ymladdedThis migration's
[tool.coverage.run](inpyproject.toml) omitsccx_keys/tests/*from coverage measurement, per this effort's standardomitpattern — a test file trivially covers itself just by running, so counting it
was inflating the reported package coverage (previously:
source = ./ccx_keyswith no
omit, soccx_keys/tests/test_ccx_keys.pyitself counted toward thepercentage). That's a one-time ~3-4% drop in the reported baseline (97.78% →
94.44%), not a regression in actual production-code coverage
(
ccx_keys/key.pyandccx_keys/__init__.pyare both 100%,ccx_keys/locator.pyis unchanged at 95%). Addedcodecov.ymlwithtarget: autoon the project check (no fixed threshold — an earlier draft added one, since
removed as unnecessary) — sibling repos already migrated in this same effort all
carry an equivalent file;
ccx-keyshad none before this PR.Known out-of-scope gap:
upgrade-python-requirements.yml.github/workflows/upgrade-python-requirements.ymlcalls the org's shared reusableworkflow, which hardcodes
ADD_PATHS="requirements". Now thatrequirements/isdeleted, that scheduled job will silently stop producing real dependency-upgrade
PRs — same gap across every repo in this migration batch; needs a fix in
openedx/.githubitself, not here.Code reviewer notes
sixremains a runtime dependency (six>=1.10.0) even though nothing inccx_keys/currently imports it — carried forward unchanged from the oldinstall_requiressince this migration doesn't touch actual dependency behavior.ccx_keys/__init__.py's__version__now reads fromimportlib.metadatainstead of being hardcoded, to avoid it going stale after the first tag-less commit (confirmed a real bug class elsewhere in this effort — a hardcoded__version__left in place after adding setuptools-scm silently drifts from the actual installed version).src/layout: verifieduv syncstill gives an importable editable install (import ccx_keysresolves tosrc/ccx_keys/__init__.py) andpython -m buildstill produces a wheel withccx_keys/*at its root (thesrc/prefix is a source-tree-only convention, not part of the installed package). This repo has no mypy config to verify against, so I could only confirm the editable-install resolution the decision was motivated by, not an actual mypy-clean run.This PR was created with Claude Code.