Repository navigation
fix(mit-ol): remove django-aqueduct from ulmo/xpro -- unresolvable - #184
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes an incompatible dependency from the deprecated Python 3.11 ulmo/xpro build cell.
Changes:
- Reverts the
django-aqueduct==0.13.0package pin, which requires Python 3.12.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
blarghmatey
added a commit
that referenced
this pull request
Aug 25, 2026
…#185) Root cause of the openassessment breakage on the lehrer PR #184 check run: the Settings Verification workflow's master/mitxonline, master/mitx, and master/mitx-staging cells have been failing intermittently for weeks with ModuleNotFoundError: No module named 'openassessment' from lms/envs/common.py's add_optional_apps() -- while the identical check on release/verawood's cells passes every time. openedx-platform's master branch has adopted uv.lock (tracked in openedx/public-engineering#552); release/verawood hasn't yet, so it still installs from requirements/edx/base.txt, which bundles ora2 (the package providing `openassessment`) unconditionally. On master, _edx_base_deps_script's uv.lock-presence branch instead runs `uv sync --no-default-groups --group assets` -- and ora2 lives in pyproject.toml's `bundled` dependency group, not `assets`. That group's own doc comment says it's "still installed by default (this group is included from `testing`, and explicitly synced in `base-requirements`)" -- master's sync arm just never asked for it, so ora2 and its dozen bundled-group siblings (edx-sga, several XBlocks, django-ses, ...) go silently missing on any release that's made the migration. add_optional_apps() only surfaces this for openassessment specifically because iterating its dotted OPTIONAL_APPS entries (openassessment.assessment, .fileupload, ...) calls importlib.util.find_spec() on a submodule whose parent package doesn't exist, which raises ModuleNotFoundError uncaught -- unlike the bare top-level entries, which the function's own try/except tolerates. The other twelve now-missing packages fail more quietly (unused, or their own absence handled elsewhere), which is why this surfaced as an "openassessment" crash specifically rather than a dozen missing-package warnings. Added `--group bundled` alongside `--group assets`, matching what the legacy `requirements/edx/base.txt` arm already provides. Verified: ruff, mypy, and the full pytest suite in tests/core/test_platform_test_helpers.py pass (14/14), including two updated assertions and inline regression context tracing back to the concrete failure. Claude-Session: https://claude.ai/code/session_01NvnwbkZqzvJMWywUbSFPMJ Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
django-aqueduct has never published a release supporting Python < 3.12 (checked every version on PyPI), but ulmo's release_python is pinned to 3.11 -- and that pin is correct: edx-platform's release/ulmo branch is only tested against Python 3.11 in its own CI matrix, so bumping ulmo to 3.12 would run it on an unsupported combination just to satisfy this one dependency. #181 added the line assuming a simple omission; running the actual build surfaced the real, unresolvable conflict: × No solution found when resolving dependencies: ╰─▶ Because the current Python version (3.11.16) does not satisfy Python>=3.12 and django-aqueduct==0.13.0 depends on Python>=3.12, we can conclude that django-aqueduct==0.13.0 cannot be used. ulmo is an unsupported Open edX release being deprecated shortly, so the corresponding Concourse jobs (build-ulmo-xpro-edxapp-image and the xpro-qa/production preview+deploy jobs, both of which depend on the ulmo image) have been paused rather than chasing a settings-loader migration for a release on its way out. Verified: schema validation passes (jsonschema against build_manifest.schema.json), pre-commit is clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NvnwbkZqzvJMWywUbSFPMJ
blarghmatey
force-pushed
the
remove-ulmo-django-aqueduct
branch
from
August 25, 2026 13:57
8b578e3 to
5516f75
Compare
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
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.
What are the relevant tickets?
N/A
Description (What does it do?)
Reverts the single line added in #181. That PR fixed a genuine omission (every sibling cell already had
django-aqueduct==0.13.0,ulmo/xpro didn't) but running the actual build afterward surfaced a real, unresolvable conflict rather than a missing-dependency bug:django-aqueducthas never published a release supporting Python < 3.12 (checked every version on PyPI).ulmo'srelease_pythonis pinned to3.11, and that pin is correct as far as it goes --edx-platform'srelease/ulmobranch is only tested against Python 3.11 in its own CI matrix (.github/workflows/unit-tests.yml), so bumpingulmoto 3.12 would run it on a combination upstream doesn't support, just to satisfy this one dependency.Per team decision:
ulmois an unsupported Open edX release being deprecated shortly, so this isn't worth a settings-loader migration for a release on its way out. The corresponding Concourse jobs (build-ulmo-xpro-edxapp-image, and thexpro-qa/xpro-productionpreview+deploy jobs, both of which depend on theulmoimage) have been paused in Concourse rather than chased further here.How can this be tested?
jsonschemaagainstbuild_manifest.schema.json.pre-commitis clean on the changed file.Additional Context
Follow-up to #181. See that PR and the linked investigation for the full trail: ol-infrastructure#5560 is what first exposed the missing dependency by wrapping LMS/CMS/celery with
opentelemetry-instrument.