Skip to content

Commit a682735

Browse files
yarikopticclaude
andcommitted
TST: Add tests for zarr upload batch retry and failure diagnostics
- test_zarr_upload_403_batch_retry_reduces_parallelism: exercises the batch-level retry loop with reduced parallelism (workers: N logged) and exponential backoff by mocking 403 responses at the request() level so _upload_zarr_file returns RETRY_NEEDED - test_zarr_upload_connection_error_diagnostics: exercises _handle_failed_items_and_raise summary logging by mocking ConnectionError on all S3 PUTs, verifying "Upload failure summary" includes exception type counts and "systematic" flag Covers code added in befe4a9 and 4a2da70. Co-Authored-By: Claude Code 2.1.81 / Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4c99954 commit a682735

1 file changed

Lines changed: 99 additions & 0 deletions

File tree

dandi/tests/test_upload.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,105 @@ def mock_request(self, method, path, **kwargs):
626626
), f"URL {url} should not have been retried but had {request_attempts[url]} attempts"
627627

628628

629+
@pytest.mark.ai_generated
630+
def test_zarr_upload_403_batch_retry_reduces_parallelism(
631+
new_dandiset: SampleDandiset,
632+
monkeypatch: pytest.MonkeyPatch,
633+
caplog: pytest.LogCaptureFixture,
634+
) -> None:
635+
"""Test that 403 errors trigger batch-level retry with reduced parallelism
636+
and exponential backoff, exercising the batch retry loop in iter_upload."""
637+
zarr_path = new_dandiset.dspath / "test.zarr"
638+
zarr.save(zarr_path, np.arange(100), np.arange(100, 0, -1))
639+
640+
# Mock at the request() level so that _upload_zarr_file sees the 403
641+
# response and returns RETRY_NEEDED, triggering the batch retry loop.
642+
request_attempts: defaultdict[str, int] = defaultdict(int)
643+
original_request = RESTFullAPIClient.request
644+
645+
def mock_request(self, method, path, **kwargs):
646+
urlpath = urlparse(path).path if path.startswith("http") else path
647+
request_attempts[urlpath] += 1
648+
649+
# Simulate 403 on first attempt for files containing "arr_1"
650+
if method == "PUT" and "arr_1" in path and request_attempts[urlpath] == 1:
651+
resp = Mock(spec=requests.Response)
652+
resp.status_code = 403
653+
resp.ok = False
654+
resp.text = "Forbidden"
655+
resp.headers = {}
656+
error = requests.HTTPError("403 Forbidden", response=resp)
657+
error.response = resp
658+
raise error
659+
660+
return original_request(self, method, path, **kwargs)
661+
662+
monkeypatch.setattr(RESTFullAPIClient, "request", mock_request)
663+
# Speed up the test by removing the exponential backoff sleep
664+
monkeypatch.setattr("dandi.files.zarr.sleep", lambda _: None)
665+
666+
with caplog.at_level("INFO", logger="dandi"):
667+
new_dandiset.upload()
668+
669+
# Verify the upload succeeded
670+
(asset,) = new_dandiset.dandiset.get_assets()
671+
assert isinstance(asset, RemoteZarrAsset)
672+
assert asset.path == "test.zarr"
673+
674+
# Verify the batch retry log message with worker count
675+
retry_msgs = [
676+
r.message for r in caplog.records if "requesting new URLs" in r.message
677+
]
678+
assert len(retry_msgs) > 0, "Expected batch retry log message"
679+
assert "workers:" in retry_msgs[0], "Expected reduced worker count in retry log"
680+
681+
682+
@pytest.mark.ai_generated
683+
def test_zarr_upload_connection_error_diagnostics(
684+
new_dandiset: SampleDandiset,
685+
monkeypatch: pytest.MonkeyPatch,
686+
caplog: pytest.LogCaptureFixture,
687+
) -> None:
688+
"""Test that ConnectionError failures produce diagnostic summary logging."""
689+
zarr_path = new_dandiset.dspath / "test.zarr"
690+
zarr.save(zarr_path, np.arange(100), np.arange(100, 0, -1))
691+
692+
# Mock put() to raise ConnectionError for all S3 uploads.
693+
# This bypasses tenacity retries (they would take too long) and directly
694+
# exercises _upload_zarr_file's except-Exception path and the
695+
# _handle_failed_items_and_raise diagnostics.
696+
original_put = RESTFullAPIClient.put
697+
698+
def mock_put(self, url, **kwargs):
699+
if "dandi-dandisets" in url:
700+
raise requests.ConnectionError(
701+
"('Connection aborted.', ConnectionAbortedError(10053, "
702+
"'An established connection was aborted by the software "
703+
"in your host machine'))"
704+
)
705+
return original_put(self, url, **kwargs)
706+
707+
monkeypatch.setattr(RESTFullAPIClient, "put", mock_put)
708+
709+
with caplog.at_level("ERROR", logger="dandi"), pytest.raises(
710+
requests.ConnectionError
711+
):
712+
new_dandiset.upload()
713+
714+
# Verify the diagnostic summary was logged
715+
summary_msgs = [
716+
r.message for r in caplog.records if "Upload failure summary" in r.message
717+
]
718+
assert (
719+
len(summary_msgs) == 1
720+
), f"Expected 1 summary message, got {len(summary_msgs)}"
721+
summary = summary_msgs[0]
722+
assert "ConnectionError" in summary
723+
assert (
724+
"systematic" in summary
725+
), "All-same-type failures should be flagged as systematic"
726+
727+
629728
@pytest.mark.ai_generated
630729
def test_upload_rejects_dandidownload_paths(
631730
new_dandiset: SampleDandiset, tmp_path: Path

0 commit comments

Comments
 (0)