ci: add caching to speed up tutor-based CI workflow - #802
ci: add caching to speed up tutor-based CI workflow#802asadali145 with Copilot wants to merge 13 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR speeds up the Tutor-based integration test workflow by adding multiple caches (Python build artifacts, pip cache, edx-platform checkout, Tutor config, and a Docker image cache in GHCR) so repeated CI runs don’t rebuild everything from scratch.
Changes:
- Resolve the edx-platform branch tip SHA up front and use it to key subsequent caches.
- Cache
dist/artifacts, pip downloads, and the edx-platform checkout to reduce repeated network/build work. - Add a GHCR-backed Docker image cache and cache Tutor config dirs to potentially skip image builds and
tutor dev launch.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot fix the failing integration tests in https://github.com/mitodl/open-edx-plugins/actions/runs/26108947537/job/76780542597, look for the reason and fix it. |
Fixed in 2895b45. The failure was in the "Push Docker image to registry cache" step: The workflow was constructing the dev image name by appending |
|
@copilot CI checks are still taking about 25 minutes on master branch. This includes about 15m for the tutor launch step. Looks like there is still some issue? Look at the latest CI run and see what can be fixed/improved. |
Investigated and fixed in 396604e. Two root causes were found: Root cause 1 — Tutor config cache never saved due to permission errors: Root cause 2 — Cache key included edx-platform SHA: Fixes:
On the next run after a cache miss, "Generate Tutor config" will take ~5 seconds and save cleanly. Subsequent runs will skip it entirely via cache hit. |
|
@copilot fix the pre-commit checks |
Fixed in b2f5f29. Two pre-commit hooks were failing:
|
|
@copilot now tests are failing with missing apps, I think tutor launch cannot be replaced with tutor config save. R&D on how tutor works with open edx and then make required changes |
Fixed in 276723d. Root cause: Without this egg-info, Fix: Added a step after |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
9fd7b9e to
6becfaa
Compare
|
@claude review this PR |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:108
- The cache key is split across two YAML lines, which introduces whitespace into the literal cache key and makes it harder to reason about cache hits/misses. Keep the key on a single line so the key value is exactly what it appears to be.
key: tutor-config-${{ steps.tutor-version.outputs.version_tag }}-${{ steps.edx-sha.outputs.branch_slug
}}
Investigated Copilot's latest review comments
The tradeoff: Looping in the team to make the call here — no code changes made pending that discussion. |
|
Could you please take a look at these findings by Claude in our current implementation to speed up the CI? I am looking for suggestion on what should we do? |
The integration-tests job held packages:write while running PR code and three sources of upstream code (edx-platform's setup.py, Tutor from git main, the image's pip installs). docker/login-action leaves the credential in ~/.docker/config.json for the remainder of the job, so the write scope was live during the image build and the test run, not just at the push step. Split the build and push into ci-image-cache.yml, which runs nightly on a schedule and touches no PR code. integration-tests now only pulls, and falls back to a local build when the cache misses. Rebuilding nightly also bounds image staleness. The openedx-dev image pins the Python dependencies of the release branch it was built against, and a pinned Tutor range is stable for months, so a cache populated only on demand would drift arbitrarily far from what the branch requires. Both workflows derive the cache tag through a shared composite action; if they disagreed on the tag the cache would silently never hit. Drop the Tutor config cache: the step it skipped takes one second, and restoring config over a later `tutor mounts add` risked masking the mount. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Snt95k6UYxUndphyNBey7S
Resolving the
|
The test step guessed the Tutor root from the matrix branch: tutor-main for master, tutor otherwise. That spelling comes from __app__, which only gains the -main suffix while tutor's main branch carries a non-empty __version_suffix__. Tutor cut v22.0.0 from main today and reset the suffix to "", so __app__ became plain "tutor", the root moved, and the master leg died on a missing env/local/docker-compose.yml. `tutor config printroot` reports the real location, and the compose project name follows from its basename, so neither has to be predicted. This also drops the hardcoded /home/runner prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Snt95k6UYxUndphyNBey7S
CI result on 581cf18, and a Tutor v22 breakage that is not from this PR
1.
|
master has failed twice on an upstream version skew, and both times fail-fast cancelled release/ulmo mid-test, so there is still no signal on whether ulmo passes. The legs test independent Open edX branches and one breaking upstream says nothing about the rest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Snt95k6UYxUndphyNBey7S
The
|
openedx-filters |
|
|---|---|
edx-platform master (the mounted source) |
3.8.0 |
release/verawood (what Tutor v22 builds) |
3.4.1 |
Tutor's main branch stopped being a nightly today: v22.0.0 was cut from it, which is the Verawood release. So the master leg now builds a Verawood image and bind-mounts master source into it. The source needs filter classes that shipped in 3.5.0–3.8.0; the image has 3.4.1.
This is the same root cause as the tutor-main path breakage — Tutor main is temporarily a release rather than a nightly — and it should resolve on its own once upstream restores the nightly suffix and resumes tracking master. It is not caused by this PR: the last green master leg was 2026-08-03, before v22.0.0 existed.
The latent bug this exposes
run_edx_integration_tests.sh already tries to install edx-platform's own pins:
if [ $CI ]; then
pip install -r ./requirements/edx/testing.txtThat would resolve the skew — master's testing.txt pulls openedx-filters==3.8.0. But it has never run. CI is set on the runner, not inside the container, and docker compose run doesn't forward it, so the guard is always false. The image's pins have therefore always had to match the mounted source by luck, which is exactly why a Tutor/platform divergence is fatal rather than self-correcting.
Worth fixing deliberately rather than as a drive-by, since making it fire changes what gets installed and adds minutes to every run. Options:
- Pass
-e CI=truein thedocker compose runinvocation so the existing branch works as intended. - Install just the skewed constraint rather than all of
testing.txt. - Leave it and accept that the
masterleg tracks Tutor's image pins, not the platform's.
I'd want a maintainer's call on that — it's orthogonal to caching, and (1) has real runtime cost.
Also pushed: fail-fast: false (4ee888c)
master has failed twice now, and both times it cancelled release/ulmo mid-test, so there is still no answer to whether ulmo passes. The legs test independent Open edX branches; one breaking upstream says nothing about the others. Small change, but without it this PR can't be evaluated while master is broken upstream.
Where the PR stands
The caching restructure itself is verified: teak is green end to end through the composite action, read-only pull, cache hit, and test run, with no packages: write anywhere in the job. The two remaining failures are both upstream Tutor v22 fallout, one of which I've fixed (printroot) and one of which needs the decision above.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Suppressed comments (7)
.github/workflows/ci-image-cache.yml:53
steps:is followed by step entries at the wrong indentation level. As written, the step list is not understeps:and the workflow will fail to parse.
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
- name: Set up Tutor
id: tutor
.github/workflows/ci-image-cache.yml:63
- The remaining step entries in this workflow also need to be indented under
steps:(currently they are aligned withsteps:). This prevents the workflow from being valid YAML for Actions.
- name: Log in to GitHub Container Registry
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
with:
registry: ghcr.io
username: ${{ github.actor }}
.github/workflows/ci-image-cache.yml:73
- The push step needs to be indented under
steps:as well; otherwise the workflow is invalid.
- name: Push image to GHCR cache
env:
LOCAL_IMAGE: ${{ steps.tutor.outputs.openedx_dev_image }}
CACHE_IMAGE: ${{ steps.tutor.outputs.cache_image_tag }}
run: |
.github/workflows/ghcr-cleanup.yml:21
steps:is followed by step entries at the wrong indentation level; the list items must be nested understeps:or GitHub Actions will fail to parse the workflow.
steps:
- name: Delete untagged openedx-dev-cache versions
uses: actions/delete-package-versions@25ad4af5be03aef10e0b3376b248688b7a44c40b # v5
with:
package-name: openedx-dev-cache
.github/workflows/ghcr-cleanup.yml:30
- This step also needs to be indented under
steps:; otherwise the workflow is invalid YAML for Actions.
- name: Trim old tagged openedx-dev-cache versions
uses: actions/delete-package-versions@25ad4af5be03aef10e0b3376b248688b7a44c40b # v5
with:
package-name: openedx-dev-cache
package-type: container
.github/workflows/ci.yml:83
- The PR description mentions caching the Tutor config directory (
~/.local/share/tutor*) and skippingtutor config saveon cache hits, but the workflow always runstutor config saveinside thesetup-tutorcomposite action and there is noactions/cachestep for the Tutor root. Either update the PR description to match the implementation, or add the missing cache restore/save so repeated runs can skip regenerating config.
- name: Set up Tutor
id: tutor
uses: ./.github/actions/setup-tutor
with:
edx_branch: ${{ matrix.edx_branch }}
ghcr_owner: ${{ env.GHCR_CACHE_OWNER }}
run_edx_integration_tests.sh:68
if [ $CI ]; thenwill error (and exit due toset -e) whenCIis unset, making the script fail in non-CI runs. Use a safe parameter expansion check so the conditional behaves correctly whenCIis missing.
echo "===== Installing uv ====="
pip install uv
if [ $CI ]; then
Update the checkout pin comment to the exact tag (v6.0.2) it resolves to, and repin delete-package-versions to the underlying commit SHA instead of the annotated tag object's SHA. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
What are the relevant tickets?
N/A
Description (What does it do?)
Adds several caching layers to
.github/workflows/ci.ymlto avoid rebuilding the Tutor environment from scratch on every push. The biggest time sinks were the Docker image build (~15–20 min), the edx-platform clone (~5–10 min), and Tutor config generation (~9–15 min).Changes:
Resolve edx-platform tip SHA early — a cheap
git ls-remotecall (no download) gives the current HEAD SHA for the branch; used as cache key across all subsequent caches.Cache built
dist/packages — keyed by a hash of all files undersrc/**,pyproject.toml, anduv.lock(including static assets and templates); skipsuv build --all-packageson hits.Cache pip packages — caches
~/.cache/pipkeyed by branch + OS; speeds up repeatedpip install tutor>=…calls.Shallow-clone edx-platform + cache the clone dir — replaces the full two-step clone+checkout with a single
git clone --depth=1 --branch=…and caches the directory keyed by branch+SHA; skips the clone entirely on hits.Docker image cache via ghcr.io — before building, tries to pull
ghcr.io/mitodl/openedx-dev-cache:<branch-sha>. On hit, retags the pulled image and skipstutor images build openedx-dev. On miss, builds normally then pushes to ghcr.io for future runs usingtutor config printvalue DOCKER_IMAGE_OPENEDX_DEVto resolve the correct image name. Push is conditioned onpushevents only (never onpull_requestruns). The GHCR namespace is hardcoded viaenv.GHCR_CACHE_OWNER: mitodlso fork PRs always pull from the upstream org's cache.Cache Tutor config directory — caches
~/.local/share/tutorand~/.local/share/tutor-mainkeyed by Tutor version + branch; skipstutor config saveentirely on hits (the docker-compose env files are already present from the cache).Generate edx-platform egg-info on the host runner — runs
pip install --no-deps -e /path/to/edx-platformafter the edx-platform is cloned/restored. When edx-platform is bind-mounted into the Tutor container, the image'sOpen_edX.egg-info/directory is overwritten by the host checkout (which has no egg-info). Without the egg-info,pkg_resourcescannot read the edx-platform'slms.djangoappentry points, soget_plugin_apps(ProjectType.LMS)fails to discover apps likecontent_libraries, causing aRuntimeErrorat Django startup. Previously,tutor dev init(viamounted-directories.sh) regenerated this egg-info inside a running container; this step reproduces that regeneration in ~5 seconds on the host without starting any containers.Estimated savings per run on cache hit:
How can this be tested?
tutor config save.ghcr.io/mitodl/openedx-dev-cachepackage is created in the org's packages after the first run on the main branch.Additional Context
packages: writepermission is declared at the job level (GitHub Actions does not support conditional job-level permissions). However, it is never exercised duringpull_requestruns: both the GHCR login step and the Docker push step are conditioned ongithub.event_name == 'push'. PRs pull from GHCR using anonymous access (the cache package is public-readable).tutor dev launch -I --skip-buildwas replaced bytutor config savebecause the former starts Docker containers (MongoDB, MySQL, etc.) that write data files owned by root. Those root-owned files causedtar: Permission deniederrors in the post-cache step, preventing the Tutor config cache from ever being saved.tutor config savegenerates all needed docker-compose files in ~5 seconds without starting any containers.Open_edX.egg-info/is generated on the host runner rather than inside a container. The egg-info directory is a portable text-format metadata directory;pkg_resourcesinside the container reads it correctly via the bind mount regardless of which Python generated it. The egg-info is also captured in the edx-platform directory cache, so on cache hits it is already present.actions/cache@v4(SHA-pinned) anddocker/login-action@v3(SHA-pinned) are the only new actions added.