Modernize event-bus-redis to uv + pyproject.toml + python-semantic-release - #249
Open
irfanuddinahmad wants to merge 18 commits into
Open
Modernize event-bus-redis to uv + pyproject.toml + python-semantic-release#249irfanuddinahmad wants to merge 18 commits into
irfanuddinahmad wants to merge 18 commits into
Conversation
Part of the org-wide src/ layout decision on openedx/public-engineering#506 (2026-07-15): flat-layout editable installs aren't reliably resolvable by tools like mypy, while src/-layout repos work correctly. Matches the approach already taken in the sibling event-bus-kafka repo (same "event bus" family, openedx/public-engineering#514). Moves edx_event_bus_redis/ to src/edx_event_bus_redis/ and updates every hardcoded reference to the old flat path: .coveragerc source, docs/conf.py (version-read path and sphinx-apidoc invocation), test_settings.py LOCALE_PATHS, Makefile translation targets, tox.ini quality-env commands, and setup.py (package_dir + find_packages(where=...) + version-read path).
Replace setup.py/setup.cfg with PEP 621 static metadata in pyproject.toml: - setuptools-scm derives the version from git tags (version_scheme = only-version, local_scheme = no-local-version, fallback_version = 0.0.0); verified the latest git tag (v1.0.0, via --sort=-v:refname) matches PyPI's actual latest published version (1.0.0), so there's no missing-tag gap for semantic-release to trip over later - license uses the SPDX expression format (license = "AGPL-3.0-or-later" + license-files), matching the identifier already used in the old setup.py - isort settings ported verbatim from setup.cfg (including the migrations skip entry) - coverage config (previously .coveragerc) moved into [tool.coverage.*], updated to the src/ layout source path, with */tests/* added to omit - __init__.py now reads __version__ via importlib.metadata instead of a hardcoded string that would go stale after every tag (grepped the repo; no other runtime code parses __version__, so this is a safe change); docs/conf.py updated to match Verified locally: editable install (uv pip install -e .) resolves the version to 1.0.0 via setuptools-scm.
Replace pip-compile / requirements/*.{in,txt} with uv and PEP 735
dependency groups:
- [dependency-groups] in pyproject.toml: test-base/test/django42 (version
matrix via [tool.uv].conflicts), quality, doc, ci, dev
- [tool.edx_lint].uv_constraints + edx_lint write_uv_constraints populate
[tool.uv].constraint-dependencies (Django<6.0, elasticsearch<7.14.0, from
edx-lint's common constraints; no repo-specific pins needed)
- uv.lock committed (119 packages resolved)
- Deleted requirements/; removed the now-stale requirements/base.in and
requirements/constraints.txt references from MANIFEST.in, and the
requirements/private.* entries from .gitignore
- tox.ini: tox-uv>=1 + uv-venv-lock-runner; dependency_groups replace deps;
quality-env commands updated for the deleted setup.py; PYTHONPATH added
to testenv/quality/pii_check so test_settings.py (still at repo root) and
test_utils/ resolve correctly under uv-venv-lock-runner, same fix needed
in the sibling event-routing-backends repo after its own src/ move
- Makefile: upgrade/requirements/quality/pii_check/test/docs/coverage/
diff_cover/test-all targets re-pointed at uv run / uv sync --group dev;
dropped the now-unused piptools target
- CI (ci.yml): astral-sh/setup-uv (SHA-pinned) replaces pip installs; added
fetch-depth: 0 so setuptools-scm can see tags; matrix job name is the
toxenv; added permissions: contents: read; workflow_call trigger added
(push:[main] removed, release.yml owns that path)
- .readthedocs.yaml: switched from 'requirements: requirements/doc.txt' to
uv sync --group doc, matching openedx/XBlock's current RTD config
- .python-version corrected from a stray "event-bus-redis" (a pre-existing
cookiecutter artifact, harmless with the old tooling but now read by uv)
to "3.12", matching requires-python
Explicitly listed all 5 django42-vs-{test,quality,doc,ci,dev} pairs in
[tool.uv].conflicts instead of relying on transitive auto-inference from a
single {test, django42} pair: with only the one explicit pair, uv's
auto-inferred conflict set for the other 4 groups was order-dependent
across separate `uv lock`/`uv sync --locked` invocations, intermittently
tripping `uv sync --locked`'s equality check in fresh tox environments
even though the inferred members were identical. Listing all 5 explicitly
removed the non-determinism (confirmed stable across 5 repeated runs).
Checked whether this repo shares event-bus-kafka's openedx-events<11.1.1
compatibility pin (that repo's test suite imports avro.tests.test_utilities
directly, which newer openedx-events dropped): it does not apply here -
this repo defines its own SubTestData0/create_simple_signal test helpers
locally in test_producer.py rather than importing them from openedx_events.
Verified by running the full suite against the open `openedx-events>=9.5.0`
range, which resolves to openedx-events 11.2.0: all 46 tests pass on both
django42 and django52. No uv_constraints pin needed.
The standard omit=["*/tests/*", ...] config drops test modules themselves
from coverage measurement (previously counted, and trivially ~100% covered
since their own lines execute when the tests run), causing a one-time,
expected ~0.2-1 point drop in reported project coverage %. Added a
documented 1% project threshold to codecov.yml, and a `# pragma: no cover`
on __init__.py's importlib.metadata PackageNotFoundError fallback (which
cannot execute while the package is installed under test) so patch
coverage stays accurate instead of flagging genuinely-unreachable code.
Verified locally: uv lock resolves cleanly; uv sync --group ci/--group dev
succeed; uv run tox passes django42, django52 (46 tests each, 99% branch
coverage with the new omit list), quality (pylint/pycodestyle/pydocstyle/
isort all clean), pii_check (100% PII annotation coverage), docs (Sphinx
build + wheel build + twine check, confirmed test files excluded from the
wheel).
Also confirmed present: .github/workflows/upgrade-python-requirements.yml
calls openedx/.github's reusable upgrade-python-requirements workflow,
which hardcodes ADD_PATHS=requirements for its PR step. Now that
requirements/ is gone, that scheduled job will keep running but silently
stop producing real dependency-upgrade PRs. This can't be fixed from this
repo; flagged in the PR description as a known gap, same as the sibling
event-bus-kafka PR (#349).
Add python-semantic-release, targeting PyPI OIDC trusted publishing: - [tool.semantic_release] in pyproject.toml: build_command uses python -m build (PSR's action environment has no uv available); major_on_zero = false, allow_zero_version = true - .github/workflows/release.yml: run_tests (reuses ci.yml via workflow_call + secrets: inherit) -> release (python-semantic-release, git_committer_name/email, changelog disabled, uploads dist/ as an artifact, exposes released/version outputs, contents: write only) -> publish_to_pypi (OIDC via id-token: write, no user/password inputs) - Deleted the old token-based .github/workflows/pypi-publish.yml (searched all workflow files for other pypi-publish/gh-action-pypi-publish triggers; this was the only one) - commitlint.yml was already present and correctly wired to the shared openedx/.github reusable workflow; left unchanged Action pins in release.yml, checked individually rather than copied wholesale, per the documented risk that SHA-pinning python-semantic-release/python-semantic-release, python-semantic-release/publish-action, actions/upload-artifact, and actions/download-artifact has produced fabricated/swapped SHAs in sibling PRs from this same effort: - python-semantic-release/python-semantic-release@v10.6.1, python-semantic-release/publish-action@v10.6.1, actions/upload-artifact@v7, actions/download-artifact@v8: left as plain version tags (verified each tag exists via `gh api repos/<owner>/<repo>/git/refs/tags/<tag>`), matching openedx/XBlock's actual, currently-releasing release.yml - pypa/gh-action-pypi-publish: pinned to the commit SHA ba38be9e461d3875417946c167d0b5f3d385a247 (v1.14.1) rather than the floating `@release/v1` branch. Verified this is the real commit SHA, not v1.14.1's annotated tag-object SHA (2834a314042ef964da07689278dd1e9d773e8afd, which 422s against the commits API and is not valid for a `uses:` pin) - via `gh api repos/pypa/gh-action-pypi-publish/git/refs/tags/v1.14.1` to get the tag object, then `.../git/tags/<that sha>` to resolve to the underlying commit. Pre-flight re-confirmed: latest git tag v1.0.0 (--sort=-v:refname) matches PyPI's actual latest published version (1.0.0), so semantic-release has no missing-tag gap to trip over on its first run. PyPI trusted publisher (OIDC) configuration for this repo could not be verified from here (requires PyPI project-owner access) - flagged as a pre-merge blocker in the PR description.
|
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
5 tasks
This repo never had a CHANGELOG.rst. Rather than disabling changelog
generation (no ticket ever required that), create one from scratch and
let python-semantic-release maintain it going forward.
- Add CHANGELOG.rst containing only the insertion marker; PSR builds
everything above it on each release
- Remove changelog: "false" from release.yml's PSR step
- Configure [tool.semantic_release.changelog] in "update" mode against
the marker, outputting RST to CHANGELOG.rst
- Set tag_format = "v{version}" to match this repo's actual tag
convention (confirmed via git tag --sort=-v:refname)
Verified locally with `uv run --with python-semantic-release
semantic-release changelog` on a branch pointed at HEAD: produces a
correctly formatted "Unreleased" section grouped by commit type, plus
backfills the v1.0.0 section from prior tag history.
python-semantic-release now computes the version and creates the release tag/PyPI publish automatically (release.yml), so the manual "Version bumped" merge-checklist item and the entire "Post merge" section (create a tag, verify tag-triggered PyPI build) no longer reflect how a release actually happens in this repo.
…atching target: auto already compares each PR against its own base commit, so the coverage threshold isn't needed and this status check isn't required for merging anyway. Also restrict git describe's tag match to this package's own vX.Y.Z tags -- the default glob also matches org-wide named-release tags (e.g. release/verawood.1), which aren't parseable as a package version and break the build whenever one of those is the closer tag.
feanil
reviewed
Aug 13, 2026
feanil
left a comment
feanil
left a comment
Contributor
There was a problem hiding this comment.
A couple of questions but generally looks good!
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."
Resolved: kept our deletion of the old pip-tools requirements/*.txt files (superseded by uv/dependency-groups).
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: The uv/pyproject.toml migration added */tests/* to [tool.coverage.run].omit (the standard modern config), which excludes test modules themselves from the coverage measurement. Those files were previously counted and are trivially ~100% covered (their own lines run whenever the tests run), so excluding them causes a one-time, expected ~1 point drop in the reported project percentage with no actual change in production-code coverage. target: auto means the new percentage becomes the baseline once this merges; no threshold was added.
mkvirtualenv -p python3.11 predates the uv migration and no longer matches pyproject.toml's requires-python (>=3.12). uv (via make requirements) creates and manages its own venv, so the manual virtualenvwrapper step is no longer needed.
Inline tox.ini's [testenv:docs]/[testenv:quality]/[testenv:pii_check] 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. tox.ini itself is untouched.
…nize-python-tooling # Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/quality.txt # requirements/test.txt
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.
…ernize-python-tooling # Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/quality.txt # requirements/test.txt
…ase.yml Brings this repo's release.yml back in line with sample-plugin's release.yml, the org's Phase 3 reference for this migration effort (openedx/public-engineering#506): - Pin python-semantic-release to the same commit SHA sample-plugin uses (v10.6.2), instead of the unpinned, out-of-date `@v10.6.1` tag reference. An unpinned action ref lets upstream silently replace what actually runs. - Pin actions/upload-artifact and actions/download-artifact to SHAs (v7.0.1 / v8.0.1) instead of unpinned floating tags, for the same reason. - Bump pypa/gh-action-pypi-publish's pin from v1.14.1 to v1.14.2 to match the reference. - Add `if-no-files-found: error` to the dist upload step so a build that produces no dist/ artifacts fails loudly instead of silently publishing nothing. - Add the missing `github.ref_name == 'main'` guard to publish_to_pypi's `if:`, matching sample-plugin's job-level guard on the default branch (defense in depth alongside the release job's own existing branch guard). - Drop `fetch-depth: 0` from the release job's checkout step. python-semantic-release auto-deepens a shallow clone itself before evaluating version history, so it was never needed here -- and sample-plugin's own release job checkout doesn't set it either. This also resolves feanil's "Why is this necessary?" review comment on this line (PR review comment 3776401525): the prior reply justified keeping it as required for PSR to walk commit history, but that justification doesn't hold given PSR's own auto-deepen behavior. All five pins above were independently re-verified via `gh api repos/<owner>/<repo>/commits/<sha>` (and, where the ref resolved to an annotated tag object, one more `git/tags/<sha>` hop) against their claimed version tags before being applied here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This repo's version is dynamic via setuptools-scm ([tool.setuptools_scm] in pyproject.toml), which already sets fallback_version = "0.0.0" -- a shallow checkout that can't resolve a real version from git history just falls back to "0.0.0" instead of failing. Nothing downstream parses __version__ into structured data that would need the real value: src/edx_event_bus_redis/__init__.py only exposes it via plain importlib.metadata.version(), docs/conf.py computes VERSION independently the same way, and the only sourcelib- related code (internal/utils.py's HEADER_SOURCELIB) is a wire-format header definition fed by caller-supplied values in tests, not by this package's own __version__ -- unlike openedx-events' EventData.sourcelib, which does parse a package's own __version__ and would have been a real reason to keep this. sample-plugin's own backend-ci.yml (the org's Phase 3 reference for this migration) doesn't set fetch-depth: 0 either. This also resolves feanil's "Why was this done?" review comment on this line (PR review comment 3776398012): the prior reply justified keeping it as a "standard setuptools-scm fix" needed for correct version resolution in CI/quality/docs runs, but that isn't actually true here given the fallback_version safety net and the lack of any consumer that needs a precise version. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both __init__.py and docs/conf.py now alias it as get_version, matching sample-plugin's convention exactly (it uses `version as get_version` in both files). Neither prior name was actually the gold-standard one -- __init__.py used the plain name, conf.py used get_distribution_version (a pkg_resources.get_distribution()-era leftover). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bmtcril
approved these changes
Sep 4, 2026
bmtcril
left a comment
bmtcril
left a comment
Contributor
There was a problem hiding this comment.
Seems reasonable to me, is this waiting on anything else?
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
5 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
event-bus-redisto uv + pyproject.toml (PEP 621/735) + python-semantic-release.Part of openedx/public-engineering#506 (tracked in openedx/public-engineering#514). This is the last repo in that 14-repo batch; its sibling
event-bus-kafka(#349, same "event bus" family, same issue) was used as a close reference and cross-checked rather than copied blindly (see notes below on where this repo genuinely differs).edx_event_bus_redis/tosrc/edx_event_bus_redis/(org-wide decision, in scope this cycle)setup.py/setup.cfgwithpyproject.toml(PEP 621 static metadata)uvwith PEP 735 dependency groups; commituv.locktox.inito usetox-uvwithuv-venv-lock-runnerastral-sh/setup-uv(SHA-pinned)python-semantic-release+release.yml, targeting PyPI OIDC trusted publishingRemoved
Deleted files:
setup.py,setup.cfg,.coveragerc,requirements/*,.github/workflows/pypi-publish.yml(replaced byrelease.yml'spublish_to_pypijob)Makefile targets:
piptoolsdropped (only existed to supportpip-sync, no longer needed).upgraderewritten to useedx_lint write_uv_constraints+uv lock --upgradeinstead ofpip-compile. All other targets (test,coverage,quality,pii_check,requirements,diff_cover,test-all,docs, translation targets) kept the same name and behavior, re-pointed atuv run/src/as needed — diffed sub-target by sub-target against the pre-migration Makefile to confirm nothing was silently dropped (e.g.test-all'squality+pii_check+tox+tox -e docssequence is unchanged).Not included
ruffmigration — per the 2026-07-15 decision on #506, ruff is out of scope for this cycle. pylint/pycodestyle/pydocstyle/isort configuration is otherwise unchanged (isort ported verbatim intopyproject.toml, including themigrationsskip entry; pycodestyle/pydocstyle stay intox.ini, which they already read from and don't supportpyproject.toml).tox.ini'senvlistdeliberately stayspy{311,312}-django{42,52}, docs, quality, unchanged from before — but notepy311-django{42,52}do not actually pass locally with the new tooling:requires-python = ">=3.12"(carried over verbatim from the oldsetup.py) makesuv syncrefuse to build an environment on Python 3.11. This is a pre-existing inconsistency, not something this migration introduced — CI's own matrix only ever exercisedpython-version: ["3.12"]and never actually ran thepy311-*factors, so this was already latent (apip install -e .under 3.11 would already have hit the sameRequires-Pythonmismatch with the old tooling). Flagging for a maintainer to decide whether to droppy311fromenvlistor bump it back — not fixed silently here since it's outside this migration's stated scope.Versioning
Dynamic, via
setuptools-scm(version_scheme = "only-version",local_scheme = "no-local-version",fallback_version = "0.0.0").__version__in__init__.pynow readsimportlib.metadatainstead of a hardcoded string (with a# pragma: no coveron thePackageNotFoundErrorfallback, which can't execute while the package is installed under test);docs/conf.pyupdated to match. Grepped the repo for other__version__consumers — none found, so the plain"0.0.0"fallback is safe.Pre-flight checked: the latest git tag (
v1.0.0, viagit tag --sort=-v:refname) matches PyPI's actual latest published version (1.0.0), so there's no missing-tag gap for semantic-release to trip over.Dependency compatibility check (openedx-events)
event-bus-kafka's equivalent PR (#349) found it needed to pinopenedx-events<11.1.1because its test suite importsavro.tests.test_utilities(SubTestData0/create_simple_signal) directly fromopenedx_events, and that module was dropped in newer releases. Since both repos share theopenedx-eventsdependency, I checked whether the same issue applies here: it does not — this repo defines its ownSubTestData0/create_simple_signaltest helpers locally intest_producer.pyrather than importing them fromopenedx_events. Verified by running the full suite with the openopenedx-events>=9.5.0range, which resolves toopenedx-events==11.2.0: all 46 tests pass on bothdjango42anddjango52. Nouv_constraintspin was needed ([tool.edx_lint].uv_constraintsis empty).Codecov
The standard
omit=["*/tests/*", ...]config excludes test modules from coverage measurement (previously counted, and trivially ~100% covered since their own lines execute whenever the tests run). This causes a small, expected drop in reported project coverage % with no actual change in production-code coverage. Addedcodecov.ymlwithtarget: autoon the project check (no fixed threshold) — it compares against each PR's own base commit, so this one-time drop becomes the new baseline once merged.Known gap:
upgrade-python-requirements.yml(pre-existing, can't be fixed from this repo).github/workflows/upgrade-python-requirements.ymlcallsopenedx/.github's shared reusable workflow, which hardcodesADD_PATHS=requirementsfor its PR-creation step. Now thatrequirements/is deleted, the scheduled job will keep running but silently produce no output (empty diff against the hardcoded path) instead of a real dependency-upgrade PR — it won't fail, so nothing will alert anyone. This can't be fixed from an individual repo's migration PR since the reusable workflow doesn't expose anadd_pathsinput. Same gap flagged in the siblingevent-bus-kafkaPR (#349) and several other repos in this effort; may be worth a follow-up issue againstopenedx/.githubto parameterizeadd_pathsor auto-detectuv.lock.Pre-merge blocker: PyPI trusted publisher (OIDC)
release.yml'spublish_to_pypijob uses OIDC (id-token: write, nouser/passwordinputs). This requires a PyPI trusted publisher to be configured for this project in PyPI's own project settings — something I cannot do or verify from this repo. Please confirm this is configured before merging, or the first automated release will fail at the publish step.Testing Notes
Verified locally at each phase, and again end-to-end after all commits, from a clean
.venv/.tox:uv lockresolves cleanly (119 packages),uv sync --group dev/--group cisucceeduv run tox -e quality,docs,pii_check,django42,django52(the envs CI actually runs) all pass: 46 tests each on django42/django52, 99% branch coverage; quality (pylint/pycodestyle/pydocstyle/isort all clean); pii_check (100% PII annotation coverage); docs (Sphinx build + wheel build +twine check, confirmed test files excluded from the wheel via[tool.setuptools.exclude-package-data])uv pip install -e .) resolves__version__to1.0.0via setuptools-scmOne thing worth calling out for review: with only a single explicit
{test, django42}pair in[tool.uv].conflicts, uv's auto-inferred conflicts for the other groups that transitively includetest(quality,doc,ci,dev) came back in a different order across separateuv lock/uv sync --lockedinvocations, intermittently trippinguv sync --locked's equality check inside freshly-created tox environments (reproduced ~50% of the time in local testing). Listing all 5{group, django42}pairs explicitly removed the non-determinism (confirmed stable across 5+ repeated runs). Flagging in case this same class of issue shows up in other repos' conflict configs.Code reviewer notes
tox.ini[testenv]/[testenv:quality]/[testenv:pii_check]sections addPYTHONPATH = {toxinidir}— needed becausetest_settings.py/test_utils/stay at the repo root (only the importable package moved tosrc/), anduv-venv-lock-runnerdoesn't puttoxinidironsys.paththe way the old runner implicitly did. Same fix was needed in the siblingevent-routing-backendsrepo after its ownsrc/move.[tool.uv].conflictsinpyproject.tomllists all 5 django42-vs-other-group pairs explicitly rather than relying on transitive inference — see testing notes above for why.This PR was created with Claude Code.