Skip to content

Commit 7e72d74

Browse files
committed
feat(uploads): harden multipart upload resilience
1 parent 28e42ce commit 7e72d74

8 files changed

Lines changed: 2684 additions & 86 deletions

‎hotdata/_upload_watchdog.py‎

Lines changed: 462 additions & 0 deletions
Large diffs are not rendered by default.

‎hotdata/uploads.py‎

Lines changed: 562 additions & 85 deletions
Large diffs are not rendered by default.

‎tests/test_upload_codex_fixes.py‎

Lines changed: 427 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 354 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,354 @@
1+
"""Integration tests for the resilient multipart path: the per-part unit driven
2+
by the re-sweep loop, exercising eager (known-size) and streaming (mint-on-demand)
3+
URL acquisition, the one-shot 403-only re-mint, 5xx → outer re-sweep, blank-ETag
4+
recovery, and finalize completeness.
5+
6+
These stub the two seams (create/finalize/mint via monkeypatch, storage PUTs via
7+
a sequencing fake pool) — no real server. The fake pool supports per-URL response
8+
*sequences* so a 500-then-200 or 403-then-200 can be scripted per path.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import threading
14+
from typing import Any, Dict, List, Optional, Tuple
15+
16+
import pytest
17+
18+
import hotdata.uploads as uploads_mod
19+
from hotdata.uploads import (
20+
MIB,
21+
MalformedSessionError,
22+
StorageError,
23+
UploadsApi,
24+
)
25+
from hotdata.models.finalize_upload_response import FinalizeUploadResponse
26+
from hotdata.models.minted_upload_part_response import MintedUploadPartResponse
27+
from hotdata.models.mint_upload_parts_response import MintUploadPartsResponse
28+
from hotdata.models.upload_session_response import UploadSessionResponse
29+
30+
31+
# --- sequencing fake storage pool -----------------------------------------
32+
33+
34+
class _Resp:
35+
def __init__(self, status: int, *, etag: Optional[str] = None, body: bytes = b""):
36+
self.status = status
37+
self._headers = {}
38+
if etag is not None:
39+
self._headers["ETag"] = etag
40+
self.data = body
41+
42+
@property
43+
def headers(self):
44+
class _H:
45+
def __init__(self, d):
46+
self._d = {k.lower(): v for k, v in d.items()}
47+
48+
def get(self, k, default=None):
49+
return self._d.get(k.lower(), default)
50+
51+
return _H(self._headers)
52+
53+
54+
class _SeqPool:
55+
"""Fake storage pool. ``responses[url]`` is a list popped per call (last
56+
entry reused once exhausted), so a 500-then-200 sequence is scriptable.
57+
Records each PUT.
58+
"""
59+
60+
def __init__(self) -> None:
61+
self.responses: Dict[str, List[_Resp]] = {}
62+
self.default = _Resp(200, etag='"default"')
63+
self.calls: List[Dict[str, Any]] = []
64+
self._lock = threading.Lock()
65+
66+
def request(self, method: str, url: str, *, body=None, headers=None, retries=None, **kw):
67+
with self._lock:
68+
self.calls.append({"method": method, "url": url, "body": body})
69+
seq = self.responses.get(url)
70+
if not seq:
71+
return self.default
72+
return seq.pop(0) if len(seq) > 1 else seq[0]
73+
74+
75+
@pytest.fixture
76+
def seq_pool(monkeypatch):
77+
pool = _SeqPool()
78+
monkeypatch.setattr(uploads_mod, "_STORAGE_POOL", pool)
79+
# The resilient path installs the watchdog ConnectionCls on the pool; the
80+
# fake doesn't need it, but install is a no-op-safe call on a non-pool, so
81+
# stub it out for the fake.
82+
monkeypatch.setattr(uploads_mod, "install_watchdog_connection_classes", lambda p: None)
83+
return pool
84+
85+
86+
def _make_api() -> UploadsApi:
87+
from hotdata import ApiClient, Configuration
88+
89+
cfg = Configuration(host="https://api.hotdata.test", api_key="k", workspace_id="ws")
90+
return UploadsApi(ApiClient(cfg))
91+
92+
93+
def _patch_finalize(monkeypatch, api, capture):
94+
def fake_finalize(upload_id, finalize_token, body=None, _request_timeout=None):
95+
from datetime import datetime, timezone
96+
97+
capture.append(("finalize", upload_id, body))
98+
return FinalizeUploadResponse(
99+
content_type=None,
100+
created_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
101+
size_bytes=0,
102+
status="ready",
103+
upload_id=upload_id,
104+
)
105+
106+
monkeypatch.setattr(api, "_finalize_handler", fake_finalize)
107+
108+
109+
def _eager_session(part_urls, part_size):
110+
return UploadSessionResponse(
111+
finalize_token="tok",
112+
headers={},
113+
mode="multipart",
114+
part_size=part_size,
115+
part_urls=part_urls,
116+
upload_id="up_eager",
117+
)
118+
119+
120+
def _streaming_session(part_size):
121+
return UploadSessionResponse(
122+
finalize_token="tok",
123+
headers={},
124+
mode="multipart",
125+
part_size=part_size,
126+
part_urls=None, # streaming: mint on demand
127+
upload_id="up_stream",
128+
)
129+
130+
131+
# --- 5xx → outer re-sweep (tier boundary, Item 5) -------------------------
132+
133+
134+
def test_part_500_recovered_by_outer_resweep(seq_pool, monkeypatch):
135+
part_size = 5 * MIB
136+
content = b"a" * part_size
137+
api = _make_api()
138+
url = "https://storage.test/p1"
139+
seq_pool.responses[url] = [_Resp(500, body=b"boom"), _Resp(200, etag='"e1"')]
140+
monkeypatch.setattr(
141+
api, "create_upload_session_handler", lambda *a, **k: _eager_session([url], part_size)
142+
)
143+
capture: List[Any] = []
144+
_patch_finalize(monkeypatch, api, capture)
145+
146+
api.upload_file(content, max_concurrency=2, round_base_delay=0.0)
147+
148+
# 500 was NOT inner-retried (would have inner-swallowed it); the outer
149+
# re-sweep re-PUT and succeeded → part PUT twice.
150+
puts = [c for c in seq_pool.calls if c["url"] == url]
151+
assert len(puts) == 2
152+
fin = capture[-1]
153+
parts = fin[2].parts
154+
assert [p.part_number for p in parts] == [1]
155+
assert parts[0].e_tag == '"e1"'
156+
157+
158+
# --- streaming: mint per part ---------------------------------------------
159+
160+
161+
def test_streaming_session_mints_per_part(seq_pool, monkeypatch):
162+
part_size = 5 * MIB
163+
content = b"b" * (2 * part_size)
164+
api = _make_api()
165+
mint_calls: List[List[int]] = []
166+
167+
def fake_mint(upload_id, x_upload_finalize_token, mint_upload_parts_request, **kw):
168+
nums = list(mint_upload_parts_request.part_numbers)
169+
mint_calls.append(nums)
170+
parts = [
171+
MintedUploadPartResponse(part_number=n, url=f"https://storage.test/spart/{n}")
172+
for n in nums
173+
]
174+
return MintUploadPartsResponse(parts=parts)
175+
176+
for n in (1, 2):
177+
seq_pool.responses[f"https://storage.test/spart/{n}"] = [_Resp(200, etag=f'"e{n}"')]
178+
monkeypatch.setattr(
179+
api, "create_upload_session_handler", lambda *a, **k: _streaming_session(part_size)
180+
)
181+
monkeypatch.setattr(api, "mint_upload_parts_handler", fake_mint)
182+
capture: List[Any] = []
183+
_patch_finalize(monkeypatch, api, capture)
184+
185+
api.upload_file(content, max_concurrency=2, round_base_delay=0.0)
186+
187+
# One mint per part, each PUT to its minted URL.
188+
assert sorted(n for nums in mint_calls for n in nums) == [1, 2]
189+
fin = capture[-1]
190+
assert [p.part_number for p in fin[2].parts] == [1, 2]
191+
assert [p.e_tag for p in fin[2].parts] == ['"e1"', '"e2"']
192+
193+
194+
def test_streaming_mint_finite_timeout(seq_pool, monkeypatch):
195+
part_size = 5 * MIB
196+
api = _make_api()
197+
seen_timeout: List[Any] = []
198+
199+
def fake_mint(upload_id, x_upload_finalize_token, mint_upload_parts_request, **kw):
200+
seen_timeout.append(kw.get("_request_timeout"))
201+
n = mint_upload_parts_request.part_numbers[0]
202+
return MintUploadPartsResponse(
203+
parts=[MintedUploadPartResponse(part_number=n, url=f"https://storage.test/s/{n}")]
204+
)
205+
206+
seq_pool.responses["https://storage.test/s/1"] = [_Resp(200, etag='"e1"')]
207+
monkeypatch.setattr(
208+
api, "create_upload_session_handler", lambda *a, **k: _streaming_session(part_size)
209+
)
210+
monkeypatch.setattr(api, "mint_upload_parts_handler", fake_mint)
211+
_patch_finalize(monkeypatch, api, [])
212+
213+
api.upload_file(b"c" * part_size, round_base_delay=0.0)
214+
# The mint must carry a finite (connect, read) timeout, not None.
215+
assert seen_timeout[0] == uploads_mod.MINT_TIMEOUT
216+
217+
218+
# --- eager 403: one-shot inline re-mint (Codex #7) ------------------------
219+
220+
221+
def test_eager_403_reminted_once_and_recovers(seq_pool, monkeypatch):
222+
part_size = 5 * MIB
223+
api = _make_api()
224+
eager_url = "https://storage.test/eager1"
225+
fresh_url = "https://storage.test/fresh1"
226+
seq_pool.responses[eager_url] = [_Resp(403, body=b"<Error>SignatureDoesNotMatch</Error>")]
227+
seq_pool.responses[fresh_url] = [_Resp(200, etag='"fresh-e1"')]
228+
229+
mint_calls: List[List[int]] = []
230+
231+
def fake_mint(upload_id, x_upload_finalize_token, mint_upload_parts_request, **kw):
232+
mint_calls.append(list(mint_upload_parts_request.part_numbers))
233+
return MintUploadPartsResponse(
234+
parts=[MintedUploadPartResponse(part_number=1, url=fresh_url)]
235+
)
236+
237+
monkeypatch.setattr(
238+
api, "create_upload_session_handler", lambda *a, **k: _eager_session([eager_url], part_size)
239+
)
240+
monkeypatch.setattr(api, "mint_upload_parts_handler", fake_mint)
241+
capture: List[Any] = []
242+
_patch_finalize(monkeypatch, api, capture)
243+
244+
api.upload_file(b"d" * part_size, round_base_delay=0.0)
245+
246+
# Exactly one inline re-mint for part 1 after the 403.
247+
assert mint_calls == [[1]]
248+
assert any(c["url"] == fresh_url for c in seq_pool.calls)
249+
assert capture[-1][2].parts[0].e_tag == '"fresh-e1"'
250+
251+
252+
def test_eager_5xx_does_not_remint(seq_pool, monkeypatch):
253+
part_size = 5 * MIB
254+
api = _make_api()
255+
eager_url = "https://storage.test/eager1"
256+
# 500 forever → never inline-remints; surfaces after the re-sweep budget.
257+
seq_pool.responses[eager_url] = [_Resp(500, body=b"boom")]
258+
259+
mint_calls: List[Any] = []
260+
monkeypatch.setattr(
261+
api,
262+
"mint_upload_parts_handler",
263+
lambda *a, **k: mint_calls.append(1) or MintUploadPartsResponse(parts=[]),
264+
)
265+
monkeypatch.setattr(
266+
api, "create_upload_session_handler", lambda *a, **k: _eager_session([eager_url], part_size)
267+
)
268+
_patch_finalize(monkeypatch, api, [])
269+
270+
with pytest.raises(StorageError) as ei:
271+
api.upload_file(b"e" * part_size, max_extra_rounds=1, round_base_delay=0.0)
272+
assert ei.value.status == 500
273+
# A 5xx is NOT an expiry signal: never inline-reminted.
274+
assert mint_calls == []
275+
276+
277+
# --- blank ETag is retryable (Item 7) -------------------------------------
278+
279+
280+
def test_blank_etag_retried_then_recovers(seq_pool, monkeypatch):
281+
part_size = 5 * MIB
282+
api = _make_api()
283+
url = "https://storage.test/p1"
284+
# First PUT returns 200 with a whitespace-only ETag → MissingETagError
285+
# (retryable); re-sweep gets a real one.
286+
seq_pool.responses[url] = [_Resp(200, etag=" "), _Resp(200, etag='"real"')]
287+
monkeypatch.setattr(
288+
api, "create_upload_session_handler", lambda *a, **k: _eager_session([url], part_size)
289+
)
290+
capture: List[Any] = []
291+
_patch_finalize(monkeypatch, api, capture)
292+
293+
api.upload_file(b"f" * part_size, round_base_delay=0.0)
294+
assert capture[-1][2].parts[0].e_tag == '"real"'
295+
296+
297+
# --- mint response validation (Item 6) ------------------------------------
298+
299+
300+
def test_storage_3xx_surfaces_as_error(seq_pool, monkeypatch):
301+
# Codex #6: with redirects disabled, a storage 3xx must surface as a
302+
# StorageError (following it would invalidate the presigned signature), not
303+
# be silently treated as success.
304+
part_size = 5 * MIB
305+
api = _make_api()
306+
url = "https://storage.test/p1"
307+
seq_pool.responses[url] = [_Resp(302, body=b"redirect")]
308+
monkeypatch.setattr(
309+
api, "create_upload_session_handler", lambda *a, **k: _eager_session([url], part_size)
310+
)
311+
_patch_finalize(monkeypatch, api, [])
312+
with pytest.raises(StorageError) as ei:
313+
api.upload_file(b"h" * part_size, max_extra_rounds=0, round_base_delay=0.0)
314+
assert ei.value.status == 302
315+
316+
317+
def test_bounded_retry_multiplication(seq_pool, monkeypatch):
318+
# The inner urllib3.Retry total and the outer re-sweep rounds MULTIPLY: a
319+
# part PUT that always fails transiently is attempted at most
320+
# (1 + max_extra_rounds) times by the OUTER loop. (The inner Retry total is a
321+
# urllib3 concern, exercised separately in test_upload_retry_policy.) Here we
322+
# bound the OUTER multiplication: a persistent 500 → exactly 1+rounds PUTs.
323+
part_size = 5 * MIB
324+
api = _make_api()
325+
url = "https://storage.test/p1"
326+
seq_pool.responses[url] = [_Resp(500, body=b"boom")] # always 500
327+
monkeypatch.setattr(
328+
api, "create_upload_session_handler", lambda *a, **k: _eager_session([url], part_size)
329+
)
330+
_patch_finalize(monkeypatch, api, [])
331+
with pytest.raises(StorageError):
332+
api.upload_file(b"i" * part_size, max_extra_rounds=2, round_base_delay=0.0)
333+
puts = [c for c in seq_pool.calls if c["url"] == url]
334+
assert len(puts) == 3 # 1 initial round + 2 re-sweeps, bounded
335+
336+
337+
def test_streaming_mint_missing_requested_part_is_rejected(seq_pool, monkeypatch):
338+
part_size = 5 * MIB
339+
api = _make_api()
340+
341+
def bad_mint(upload_id, x_upload_finalize_token, mint_upload_parts_request, **kw):
342+
# Return a DIFFERENT part number than requested.
343+
return MintUploadPartsResponse(
344+
parts=[MintedUploadPartResponse(part_number=999, url="https://storage.test/x")]
345+
)
346+
347+
monkeypatch.setattr(
348+
api, "create_upload_session_handler", lambda *a, **k: _streaming_session(part_size)
349+
)
350+
monkeypatch.setattr(api, "mint_upload_parts_handler", bad_mint)
351+
_patch_finalize(monkeypatch, api, [])
352+
353+
with pytest.raises(MalformedSessionError):
354+
api.upload_file(b"g" * part_size, max_extra_rounds=0, round_base_delay=0.0)

0 commit comments

Comments
 (0)