Skip to content

Commit f95c79c

Browse files
quazi-hCopilot
andcommitted
fix: Prevent infinite loop in legacy edx course export op
The export_edx_courses op in the legacy IRx pipeline had two bugs that caused the courses/ folder to silently go missing in the S3 data packages (observed since ~2026-01-21 for mitx-etl-mitxonline-production): 1. Infinite loop in the status-polling while loop When check_course_export_status() raised httpx.HTTPStatusError (e.g., HTTP 404 from django-user-tasks after record cleanup, or a deleted course), the bare 'continue' swallowed the error without adding the course to failed_exports. The while-loop condition len(successful U failed) < len(tasks) was never satisfied, so the job hung indefinitely and the courses/ folder was never written. Fix: catch HTTPStatusError, log the status code and course id, and add the course to failed_exports so the loop can terminate. Also add a 60-minute hard timeout (matching the openedx asset-based code) and skip courses already resolved to avoid redundant API calls. 2. Abort on partial-failure from the export trigger API The ol_openedx_course_export plugin returns HTTP 400 when any course fails to queue, even if other courses succeeded. The unconditional raise_for_status() call caused export_courses() to raise immediately, aborting the entire op before any successfully-queued tarballs could be copied. Fix: allow HTTP 400 through so the caller receives the response body (upload_task_ids + failed_uploads) and can still process the courses that were queued successfully. Log the initially-failed courses as a warning. Fixes: mitodl/hq#11104 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent efcd756 commit f95c79c

2 files changed

Lines changed: 40 additions & 3 deletions

File tree

  • dg_projects/legacy_openedx/legacy_openedx/ops
  • packages/ol-orchestrate-lib/src/ol_orchestrate/resources

dg_projects/legacy_openedx/legacy_openedx/ops/open_edx.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
from pypika import MySQLQuery as Query
3232
from pypika import Table, Tables
3333

34+
COURSE_EXPORT_TIMEOUT = timedelta(minutes=60)
35+
3436

3537
class ListCoursesConfig(Config):
3638
edx_course_api_page_size: int = Field(
@@ -577,22 +579,47 @@ def export_edx_courses(
577579
exported_courses = context.resources.openedx.export_courses(
578580
course_ids=edx_course_ids,
579581
)
582+
failed_initial = exported_courses.get("failed_uploads", {})
583+
if failed_initial:
584+
context.log.warning(
585+
"The following courses failed to queue for export and will be skipped: %s",
586+
list(failed_initial.keys()),
587+
)
580588
successful_exports: set[str] = set()
581589
failed_exports: set[str] = set()
582590
tasks = exported_courses["upload_task_ids"]
583591
context.log.info("Exporting %s tasks from Open edX", len(tasks))
584592
# Possible status values found here:
585593
# https://github.com/openedx/django-user-tasks/blob/master/user_tasks/models.py
594+
start_time = datetime.now(tz=UTC)
586595
while len(successful_exports.union(failed_exports)) < len(tasks):
596+
if datetime.now(tz=UTC) - start_time > COURSE_EXPORT_TIMEOUT:
597+
timed_out = set(tasks.keys()) - successful_exports - failed_exports
598+
context.log.error(
599+
"Timed out waiting for course exports after %s. Unresolved courses: %s",
600+
COURSE_EXPORT_TIMEOUT,
601+
timed_out,
602+
)
603+
break
587604
time.sleep(timedelta(seconds=60).seconds)
588605
for course_id, task_id in tasks.items():
606+
if course_id in successful_exports or course_id in failed_exports:
607+
continue
589608
try:
590609
task_status = context.resources.openedx.check_course_export_status(
591610
course_id,
592611
task_id,
593612
)
594-
except httpx.HTTPStatusError:
595-
# Don't fail the whole job if one task status yields an error
613+
except httpx.HTTPStatusError as e:
614+
context.log.warning(
615+
"HTTP %s error checking export status for course %s "
616+
"(task %s), marking as failed: %s",
617+
e.response.status_code,
618+
course_id,
619+
task_id,
620+
e,
621+
)
622+
failed_exports.add(course_id)
596623
continue
597624
if task_status["state"] == "Succeeded":
598625
successful_exports.add(course_id)

packages/ol-orchestrate-lib/src/ol_orchestrate/resources/openedx.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ def export_courses(self, course_ids: list[str]) -> dict[str, dict[str, str]]:
6464
6565
:returns: A dictionary of course IDs and the S3 URL where it will be exported
6666
to.
67+
68+
.. note::
69+
The ol_openedx_course_export plugin returns HTTP 400 when some (but
70+
not all) courses fail to queue. We intentionally allow 400 through so
71+
that callers can still process the successfully-queued tasks from the
72+
``upload_task_ids`` field and inspect ``failed_uploads`` themselves.
73+
Any other non-2xx status is still raised as an error.
6774
"""
6875
request_url = f"{self.studio_url}/api/courses/v0/export/"
6976
response = self.http_client.post(
@@ -72,7 +79,10 @@ def export_courses(self, course_ids: list[str]) -> dict[str, dict[str, str]]:
7279
headers={"Authorization": f"JWT {self._fetch_access_token()}"},
7380
timeout=60,
7481
)
75-
response.raise_for_status()
82+
# HTTP 400 is returned on partial failure (some courses failed to queue);
83+
# allow the caller to handle failed_uploads gracefully.
84+
if response.status_code != 400: # noqa: PLR2004
85+
response.raise_for_status()
7686
return response.json()
7787

7888
def check_course_export_status(

0 commit comments

Comments
 (0)