fix(openedx-client): send only the next page's query, and survive a bare 429

[blarghmatey]

What are the relevant tickets?

Sentry: DAGSTER-E

Description (What does it do?)

Part of a set of PRs working through the Sentry backlog from #2518.

The Sentry issue is an AttributeError in the 429 retry handler, but that turned out to be the second bug on this request path — the first one is what produced the 429.

1. Pagination sent the whole URL as a parameter name.

get_edx_course_ids passed the absolute URL from pagination.next straight to parse_qs:

extra_params=parse_qs(next_page)

parse_qs on an absolute URL returns a single entry whose key is everything up to the first =:

>>> parse_qs("https://courses.learn.mit.edu/api/courses/v1/courses/?page=2")
{'https://courses.learn.mit.edu/api/courses/v1/courses/?page': ['2']}

So the intended page parameter was never sent as page, and each request carried a URL-shaped junk parameter instead. Because the API echoes the request back when building next, the query string grew every round trip. The request URL in the Sentry event has the courses endpoint nested roughly 25 times before ending in &page=27. That is what drew the rate limit.

Parsing urlparse(next_page).query sends page as a real parameter and keeps the query string flat.

2. The 429 handler crashed on a 429 with no Retry-After.

retry_after = error_response.response.headers.get("Retry-After", 60)   # int!
delay = int(retry_after) if retry_after.isdigit() else 60              # AttributeError

The default was the int 60, so .isdigit() raised AttributeError: 'int' object has no attribute 'isdigit' on exactly the responses this branch exists to handle. A recoverable rate limit became a hard job failure. The default is now the string "60".

How can this be tested?

cd packages/ol-orchestrate-lib && uv run pytest tests/ -q
# 80 passed, 80 skipped

New tests:

• tests/resources/test_openedx_client.py — walks three pages against a DRF-shaped mock (next is absolute and echoes the request). Asserts page arrives as page, and that no parameter name is a URL, which is the signature of the bug.
• tests/resources/test_oauth.py::test_rate_limit_retry_survives_a_missing_retry_after_header — 429 with no Retry-After, then 200; asserts a 60s backoff was taken instead of raising.

Both were verified to fail against the unfixed code. Worth noting the pagination test doesn't merely fail on the old code — against a conformant mock the old code loops forever, because page never reaches the server so every response is page 1 with a next.

pre-commit run --files <changed> is clean (ruff format, ruff check, mypy).

Additional Context

The same parse_qs(next_page) pattern appears twice more in the same class, in get_edxorg_programs and get_edxorg_mitx_courses, both walking discovery.edx.org. I left them alone to keep this PR scoped to the path with production evidence behind it — happy to fix them here instead if you'd prefer them together. They will have the same behaviour.

[Copilot]

Pull request overview

Fixes Open edX pagination and rate-limit recovery failures.

Changes:

• Parses only the query component of pagination URLs.
• Handles missing Retry-After headers safely.
• Adds regression tests for both cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
packages/ol-orchestrate-lib/src/ol_orchestrate/resources/openedx.py	Corrects pagination query parsing.
packages/ol-orchestrate-lib/src/ol_orchestrate/resources/oauth.py	Prevents headerless 429 retries from crashing.
packages/ol-orchestrate-lib/tests/resources/test_openedx_client.py	Tests multi-page traversal and termination.
packages/ol-orchestrate-lib/tests/resources/test_oauth.py	Tests retry behavior without Retry-After.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.