diff --git a/hermes_cli/diagnostics_upload.py b/hermes_cli/diagnostics_upload.py index 6ec4d66edc8..34f1378ffea 100644 --- a/hermes_cli/diagnostics_upload.py +++ b/hermes_cli/diagnostics_upload.py @@ -10,8 +10,12 @@ Google-OAuth-gated viewer. Flow: 1. POST {NAS_BASE}/api/diagnostics/upload-url → {uploadUrl, viewUrl, id, ...} + (the request body carries ``sizeBytes``; NAS signs it into the presigned + URL's ``ContentLength``, so the PUT must send exactly that many bytes) 2. PUT (the gzipped bundle, Content-Type application/gzip) - 3. POST {NAS_BASE}/api/diagnostics/confirm (best-effort; failures swallowed) + +NAS is stateless — the object's existence in S3 is the only state, so there is +no confirm/callback step. Uses stdlib ``urllib`` only, matching ``debug.py`` style — no third-party deps. """ @@ -115,40 +119,15 @@ def put_bundle( raise RuntimeError(f"diagnostics bundle PUT failed: HTTP {status}") -def confirm_upload(upload_id: str, size_bytes: int) -> None: - """Best-effort notify NAS that the upload completed. - - Purely advisory — the object's existence in S3 is the source of truth - (NAS is stateless per the design), so any failure here is swallowed and - must NOT abort a share that already succeeded. - """ - try: - payload = json.dumps( - {"id": upload_id, "sizeBytes": int(size_bytes)} - ).encode("utf-8") - req = urllib.request.Request( - f"{NAS_BASE}/api/diagnostics/confirm", - data=payload, - method="POST", - headers={ - "Content-Type": "application/json", - "User-Agent": _USER_AGENT, - }, - ) - with urllib.request.urlopen(req, timeout=_REQUEST_TIMEOUT): - pass - except Exception: - # Confirm is advisory only — never let it surface to the caller. - pass - - def share_to_nous(report_bundle: bytes) -> dict: """Orchestrate the full Nous-S3 upload of a gzipped *report_bundle*. - Returns the dict from :func:`request_upload_url` (which carries - ``viewUrl`` / ``id`` / expiry metadata) so the caller can print the - viewer link. Raises on any failure of the required steps (mint URL or - PUT); the confirm step is best-effort and never raises. + Two steps: mint a presigned PUT URL (sending the exact ``sizeBytes`` NAS + signs into the URL's ``ContentLength``), then PUT the bundle. NAS is + stateless — the object's existence in S3 is the only state, so there is no + confirm/callback step. Returns the dict from :func:`request_upload_url` + (which carries ``viewUrl`` / ``id`` / expiry metadata) so the caller can + print the viewer link. Raises on any failure of either step. """ size_bytes = len(report_bundle) info = request_upload_url( @@ -156,8 +135,4 @@ def share_to_nous(report_bundle: bytes) -> dict: ) put_bundle(info["uploadUrl"], report_bundle, content_type="application/gzip") - upload_id = info.get("id") - if upload_id: - confirm_upload(upload_id, size_bytes) - return info diff --git a/tests/hermes_cli/test_diagnostics_upload.py b/tests/hermes_cli/test_diagnostics_upload.py index a9b4bbe7a6d..b2c5c87635b 100644 --- a/tests/hermes_cli/test_diagnostics_upload.py +++ b/tests/hermes_cli/test_diagnostics_upload.py @@ -176,43 +176,12 @@ class TestPutBundle: put_bundle("https://u", b"data") -# --------------------------------------------------------------------------- -# confirm_upload -# --------------------------------------------------------------------------- - -class TestConfirmUpload: - def test_confirm_posts(self): - from hermes_cli.diagnostics_upload import confirm_upload - - resp = _resp(status=200, body=b"") - with patch( - "hermes_cli.diagnostics_upload.urllib.request.urlopen", - return_value=resp, - ) as urlopen: - confirm_upload("abc-123", 1024) - req = urlopen.call_args[0][0] - assert req.method == "POST" - assert req.full_url.endswith("/api/diagnostics/confirm") - sent = json.loads(req.data.decode()) - assert sent == {"id": "abc-123", "sizeBytes": 1024} - - def test_confirm_failure_is_swallowed(self): - from hermes_cli.diagnostics_upload import confirm_upload - - with patch( - "hermes_cli.diagnostics_upload.urllib.request.urlopen", - side_effect=urllib.error.URLError("connection refused"), - ): - # Must NOT raise. - confirm_upload("abc-123", 1024) - - # --------------------------------------------------------------------------- # share_to_nous (orchestration) # --------------------------------------------------------------------------- class TestShareToNous: - def test_orchestrates_request_put_confirm(self): + def test_orchestrates_request_then_put(self): from hermes_cli import diagnostics_upload as mod info = { @@ -224,38 +193,35 @@ class TestShareToNous: blob = b"gzipped-bundle" with patch.object(mod, "request_upload_url", return_value=info) as req, \ - patch.object(mod, "put_bundle") as put, \ - patch.object(mod, "confirm_upload") as confirm: + patch.object(mod, "put_bundle") as put: result = mod.share_to_nous(blob) assert result == info req.assert_called_once() - # request was told the real byte size + # request was told the real byte size (NAS signs it into ContentLength) assert req.call_args.kwargs["size_bytes"] == len(blob) # PUT got the signed URL + the exact blob put.assert_called_once_with( info["uploadUrl"], blob, content_type="application/gzip" ) - confirm.assert_called_once_with("id-9", len(blob)) def test_put_failure_propagates(self): from hermes_cli import diagnostics_upload as mod info = {"id": "id-9", "uploadUrl": "https://u", "viewUrl": "v"} with patch.object(mod, "request_upload_url", return_value=info), \ - patch.object(mod, "put_bundle", side_effect=RuntimeError("PUT failed")), \ - patch.object(mod, "confirm_upload") as confirm: + patch.object(mod, "put_bundle", side_effect=RuntimeError("PUT failed")): with pytest.raises(RuntimeError): mod.share_to_nous(b"data") - # confirm never runs if PUT failed - confirm.assert_not_called() - def test_confirm_skipped_without_id(self): + def test_share_succeeds_without_id_in_response(self): from hermes_cli import diagnostics_upload as mod + # NAS is stateless and there is no confirm step, so the share must + # succeed regardless of whether the response carries an ``id``. info = {"uploadUrl": "https://u", "viewUrl": "v"} # no id with patch.object(mod, "request_upload_url", return_value=info), \ - patch.object(mod, "put_bundle"), \ - patch.object(mod, "confirm_upload") as confirm: - mod.share_to_nous(b"data") - confirm.assert_not_called() + patch.object(mod, "put_bundle") as put: + result = mod.share_to_nous(b"data") + assert result == info + put.assert_called_once()