Track contract run edX clones and make the clone safe to retry (C3 1/3) - #3963
Merged
Merged
Conversation
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Target lookup errors and concurrent task deliveries can still misclassify or duplicate clones.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds persistent tracking and retry safety for B2B Open edX course-run cloning.
Changes:
- Adds
CourseRunClonestatus, attempt, timestamp, and error tracking. - Makes clone retries resume when the target was previously requested.
- Creates pending tracking records before queuing contract clones.
File summaries
| File | Description |
|---|---|
openedx/tasks.py |
Tracks clone attempts and outcomes. |
openedx/tasks_test.py |
Tests task status transitions. |
openedx/models.py |
Defines clone tracking model. |
openedx/migrations/0012_add_course_run_clone.py |
Creates tracking table. |
openedx/constants.py |
Defines clone statuses. |
openedx/api.py |
Adds retry-aware cloning behavior. |
openedx/api_test.py |
Tests collision and request-stamp behavior. |
b2b/api.py |
Creates pending records before queueing. |
b2b/api_test.py |
Verifies pending-record creation. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
blarghmatey
force-pushed
the
b2b-c3-clone-status
branch
from
September 15, 2026 14:53
0aa67a9 to
ff09a33
Compare
jkachel
approved these changes
Sep 16, 2026
jkachel
left a comment
jkachel
left a comment
Contributor
There was a problem hiding this comment.
Seems fine - tests check out and read-through made sense. I lack a working edX integration at the moment but the touchpoints for this also don't really change.
blarghmatey
force-pushed
the
b2b-c3-clone-status
branch
4 times, most recently
from
September 17, 2026 18:14
6965bef to
612b3eb
Compare
blarghmatey
force-pushed
the
b2b-c3-clone-status
branch
from
September 22, 2026 13:51
612b3eb to
331aca5
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
A contract run whose edX clone exhausted its retries left a local run and product with no course behind it, and nothing recorded that. The staff contract API needs to report and retry that, and runs created by the management commands or Wagtail need it too, so the record is written by clone_courserun itself rather than by any one caller. process_course_run_clone raised ValueError whenever the target already existed in edX, and ValueError is not retried. A retry after edX had accepted the clone therefore failed permanently. The clone record now stamps clone_requested_at before asking edX to clone, and an existing target is accepted only when that stamp is set, so a real key collision still fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy
…rget clone_courserun is acks_late, so a redelivered message could run two attempts for one run at once. Both could see the target absent and ask edX to clone, and the losing attempt, holding a record loaded before the winner stamped clone_requested_at, could mark the run failed after it succeeded. Attempts now take a per-run cache lock, the same pattern create_program_contract_runs uses. A delivery that finds the lock held leaves the record alone, and the record is loaded only after the lock is taken. start_attempt increments attempts with F(). process_course_run_clone treated every CourseRunAPIError from the target lookup as "not in edX". edx_api raises it for any HTTP error, so a 401 or 5xx would stamp the record and send a clone request, and a later retry would then accept an existing course as this run's clone. Only a 404 on the wrapped HTTPError now means absent. Anything else is re-raised and retried by the task. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy
blarghmatey
force-pushed
the
b2b-c3-clone-status
branch
from
September 25, 2026 16:38
331aca5 to
1af91bc
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?
Capability C3 of the B2B onboarding RFC, https://github.com/mitodl/hq/discussions/12784. Design and decisions: https://github.com/mitodl/hq/discussions/12784#discussioncomment-18437846.
Stack (1 of 3). Based on
main. 2/3 moves contract setup out of the management commands. 3/3 adds the staff contract API, which reads the record added here.Description (What does it do?)
A contract run's edX clone runs in Celery (
clone_courserun). When it exhausted its retries, the MITx Online run and product stayed behind with no course in edX. The task logged the exception, but nothing in MITx Online recorded the failure. The staff contract API reports and retries clones, so the state has to be stored somewhere.CourseRunClone(openedx/models.py, migration0012_add_course_run_clone): one row per course run withstatus(pending,cloning,cloned,failed),attempts,clone_requested_atand the lasterror.clone_courserunitself (get_or_create), so clones queued by the management commands, the Wagtail program inline or the API are all tracked. The error is recorded before each retry. The status becomesfailedwhen retries run out, or on an exception that isn't retried.create_contract_runcreates thependingrow before queueing, so a run has a status before a worker picks the task up.process_course_run_cloneraisedValueErrorwhenever the target already existed in edX, andValueErrorisn't retried, so a retry after edX had accepted the clone failed permanently. It now stampsclone_requested_atjust before asking edX to clone. An existing target counts as this run's only when the stamp is set: the clone call is skipped and run data and modes are still pushed. Without the stamp an existing target still raises, so a real key collision fails as before.courses/api.pycallsprocess_course_run_clonedirectly, without a record, and is unchanged. Runs cloned before this deploys have no record. Setup status (3/3) shows them with no clone status rather than as pending.How can this be tested?
Run in the web container:
pytest openedx/tasks_test.py openedx/api_test.py b2b/api_test.py b2b/models_test.py b2b/management/tests courses/api_test.py, 615 passed and 7 skipped.makemigrations --checkreports no changes. pre-commit passes.New tests:
openedx/tasks_test.py: the record through a retry, retry exhaustion, a non-retried error, and success with and without a pre-created rowopenedx/api_test.py: an existing target is accepted only with the stamp, and the stamp is written before the clone callb2b/api_test.py:create_contract_runcreates the pending rowNot run: a clone against a real edX. With edX running locally,
b2b_courseware add <contract> <course readable_id>should leave aCourseRunClonerow for the new run that moves tocloned.🤖 Generated with Claude Code
https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy