Skip to content

Commit f6e89ff

Browse files
praann07romanlutz
andauthored
FIX: add a request timeout to remote dataset downloads (#3154)
Co-authored-by: Roman Lutz <romanlutz13@gmail.com>
1 parent a2398d3 commit f6e89ff

2 files changed

Lines changed: 31 additions & 2 deletions

File tree

‎pyrit/datasets/seed_datasets/remote/remote_dataset_loader.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ class _RemoteDatasetLoader(SeedDatasetProvider, ABC):
5454
and cached downloads.
5555
"""
5656

57+
# Seconds to wait for a remote server before giving up, so a stalled download cannot hang forever.
58+
REQUEST_TIMEOUT_SECONDS: ClassVar[int] = 60
59+
5760
FILE_TYPE_HANDLERS: ClassVar[dict[str, dict[str, Callable[..., Any]]]] = {
5861
"json": {"read": read_json, "write": write_json},
5962
"jsonl": {"read": read_jsonl, "write": write_jsonl},
@@ -285,7 +288,7 @@ def _fetch_from_public_url(self, *, source: str, file_type: str) -> list[dict[st
285288
ValueError: If the file_type is invalid.
286289
Exception: If the request to fetch examples fails.
287290
"""
288-
response = requests.get(source)
291+
response = requests.get(source, timeout=self.REQUEST_TIMEOUT_SECONDS)
289292
if response.status_code == 200:
290293
if file_type in self.FILE_TYPE_HANDLERS:
291294
if file_type == "json":
@@ -528,7 +531,7 @@ def _download_and_parse() -> dict[str, list[dict[str, Any]]]:
528531
temp_to_clean = zip_path
529532

530533
logger.info(f"Downloading zip archive from {source}")
531-
with requests.get(source, stream=True) as response:
534+
with requests.get(source, stream=True, timeout=self.REQUEST_TIMEOUT_SECONDS) as response:
532535
response.raise_for_status()
533536
with zip_path.open("wb") as fh:
534537
for chunk in response.iter_content(chunk_size=1 << 16):

‎tests/unit/datasets/test_remote_dataset_loader.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,15 @@ def test_fetch_from_public_url_non_json_file_type(self):
255255
result = loader._fetch_from_public_url(source="https://example.com/data.csv", file_type="csv")
256256
assert result == [{"header1": "value1", "header2": "value2"}]
257257

258+
def test_fetch_from_public_url_passes_request_timeout(self):
259+
loader = ConcreteRemoteLoader()
260+
mock_response = MagicMock()
261+
mock_response.status_code = 200
262+
mock_response.text = '[{"key": "value"}]'
263+
with patch("requests.get", return_value=mock_response) as mock_get:
264+
loader._fetch_from_public_url(source="https://example.com/data.json", file_type="json")
265+
assert mock_get.call_args.kwargs["timeout"] == loader.REQUEST_TIMEOUT_SECONDS
266+
258267
def test_fetch_from_public_url_invalid_file_type_raises(self):
259268
loader = ConcreteRemoteLoader()
260269
mock_response = MagicMock()
@@ -318,6 +327,23 @@ async def test_parses_multiple_inner_files(self, tmp_path, monkeypatch):
318327
assert result["folder/a.jsonl"] == [{"a": 1}, {"a": 2}]
319328
assert result["folder/b.jsonl"] == [{"b": 3}]
320329

330+
async def test_zip_download_passes_request_timeout(self, tmp_path, monkeypatch):
331+
monkeypatch.setattr(
332+
"pyrit.datasets.seed_datasets.remote.remote_dataset_loader.DB_DATA_PATH",
333+
tmp_path,
334+
)
335+
zip_bytes = self._make_zip_bytes({"x.json": '[{"k": "v"}]'})
336+
337+
mock_get = MagicMock(return_value=self._mock_streaming_response(zip_bytes))
338+
with patch(
339+
"pyrit.datasets.seed_datasets.remote.remote_dataset_loader.requests.get",
340+
mock_get,
341+
):
342+
loader = ConcreteRemoteLoader()
343+
await loader._fetch_zip_from_url_async(source=self.SOURCE, inner_files=["x.json"], cache=False)
344+
345+
assert mock_get.call_args.kwargs["timeout"] == loader.REQUEST_TIMEOUT_SECONDS
346+
321347
async def test_caches_zip_on_disk(self, tmp_path, monkeypatch):
322348
monkeypatch.setattr(
323349
"pyrit.datasets.seed_datasets.remote.remote_dataset_loader.DB_DATA_PATH",

0 commit comments

Comments
 (0)