Pre-existing gap in the media download engine, surfaced by the #39 mutation review (not introduced by #39).
The cascade that picks the "worst" failure reason across a photo's download attempts (_worse in media.py) does not pin the "5xx beats 4xx" severity ordering: a transient server error (5xx) should be reported as more severe than a client error (4xx) when both occur across the orig/large cascade, so ret/alerting logic keys off the right reason.
Do
- Add a test
test_download_all_mixed_cascade_reports_more_severe_reason: orig attempt → 404, large attempt → 503 ⇒ the reported failure_reason is the 5xx/transient one (http_5xx / transient), not http_4xx.
- Pin the ordering in
_worse if the test exposes it as unpinned.
Note
The broader _attach_x_sources / re-enrich-key hardening this review also touched belongs with the existing #58, not here — this issue is scoped to the _worse severity ordering only.
Refs: #39, #65, #58.
Pre-existing gap in the media download engine, surfaced by the #39 mutation review (not introduced by #39).
The cascade that picks the "worst" failure reason across a photo's download attempts (
_worseinmedia.py) does not pin the "5xx beats 4xx" severity ordering: a transient server error (5xx) should be reported as more severe than a client error (4xx) when both occur across the orig/large cascade, so ret/alerting logic keys off the right reason.Do
test_download_all_mixed_cascade_reports_more_severe_reason: orig attempt → 404, large attempt → 503 ⇒ the reportedfailure_reasonis the 5xx/transient one (http_5xx/ transient), nothttp_4xx._worseif the test exposes it as unpinned.Note
The broader
_attach_x_sources/ re-enrich-key hardening this review also touched belongs with the existing #58, not here — this issue is scoped to the_worseseverity ordering only.Refs: #39, #65, #58.