Skip to content

Commit a810272

Browse files
committed
address feedback
1 parent 6a22ea5 commit a810272

File tree

4 files changed

+27
-14
lines changed

4 files changed

+27
-14
lines changed

src/apify_client/_http_clients/_base.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,15 @@ def _parse_params(params: dict[str, Any] | None) -> dict[str, Any] | None:
168168
def _compute_timeout(self, timeout: Timeout, attempt: int) -> int | float | None:
169169
"""Resolve a timeout tier and compute the timeout for a request attempt with exponential increase.
170170
171-
For `'no_timeout'`, returns `None`. For tier literals and explicit `timedelta` values, doubles the timeout
172-
with each attempt but caps at `timeout_max`.
171+
For `no_timeout`, returns `None` to indicate no timeout. For tier literals and explicit `timedelta` values,
172+
doubles the timeout with each attempt but caps at `timeout_max`.
173173
174174
Args:
175175
timeout: The timeout specification to resolve (tier literal or explicit `timedelta`).
176176
attempt: Current attempt number (1-indexed).
177177
178178
Returns:
179-
Timeout in seconds, or `None` for `'no_timeout'`.
179+
Timeout in seconds, or `None` for no timeout.
180180
"""
181181
if timeout == 'no_timeout':
182182
return None

src/apify_client/_http_clients/_impit.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,18 @@ def _make_request(
200200
try:
201201
url_with_params = self._build_url_with_params(url, params)
202202

203+
# Impit treats timeout=None as "use client default (30s)", not "no timeout".
204+
# Use a large value (24 hours) to effectively disable the timeout.
205+
# This can be removed once impit updates its behaviour: https://github.com/apify/impit/issues/401
206+
computed_timeout = self._compute_timeout(timeout, attempt)
207+
impit_timeout = 86_400 if computed_timeout is None else computed_timeout
208+
203209
response = self._impit_client.request(
204210
method=method,
205211
url=url_with_params,
206212
headers=headers,
207213
content=content,
208-
timeout=self._compute_timeout(timeout, attempt),
214+
timeout=impit_timeout,
209215
stream=stream or False,
210216
)
211217

@@ -436,12 +442,18 @@ async def _make_request(
436442
try:
437443
url_with_params = self._build_url_with_params(url, params)
438444

445+
# Impit treats timeout=None as "use client default (30s)", not "no timeout".
446+
# Use a large value (24 hours) to effectively disable the timeout.
447+
# This can be removed once impit updates its behaviour: https://github.com/apify/impit/issues/401
448+
computed_timeout = self._compute_timeout(timeout, attempt)
449+
impit_timeout = 86_400 if computed_timeout is None else computed_timeout
450+
439451
response = await self._impit_async_client.request(
440452
method=method,
441453
url=url_with_params,
442454
headers=headers,
443455
content=content,
444-
timeout=self._compute_timeout(timeout, attempt),
456+
timeout=impit_timeout,
445457
stream=stream or False,
446458
)
447459

src/apify_client/_resource_clients/request_queue.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,7 @@ async def _batch_add_requests_worker(
828828
self,
829829
queue: asyncio.Queue[Iterable[dict]],
830830
request_params: dict,
831+
timeout: Timeout,
831832
) -> BatchAddResponse:
832833
"""Worker function to process a batch of requests.
833834
@@ -852,7 +853,7 @@ async def _batch_add_requests_worker(
852853
method='POST',
853854
params=request_params,
854855
json=list(request_batch),
855-
timeout='medium',
856+
timeout=timeout,
856857
)
857858

858859
result = response_to_dict(response)
@@ -879,7 +880,7 @@ async def batch_add_requests(
879880
max_parallel: int = 5,
880881
max_unprocessed_requests_retries: int | None = None,
881882
min_delay_between_unprocessed_requests_retries: timedelta | None = None,
882-
timeout: Timeout = 'medium', # noqa: ARG002
883+
timeout: Timeout = 'medium',
883884
) -> BatchAddResult:
884885
"""Add requests to the request queue in batches.
885886
@@ -926,7 +927,7 @@ async def batch_add_requests(
926927
async with asyncio.TaskGroup() as tg:
927928
workers = [
928929
tg.create_task(
929-
self._batch_add_requests_worker(asyncio_queue, request_params),
930+
self._batch_add_requests_worker(asyncio_queue, request_params, timeout),
930931
name=f'batch_add_requests_worker_{i}',
931932
)
932933
for i in range(max_parallel)

tests/unit/test_client_timeouts.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,24 @@ async def mock_request_async(*args: Any, **kwargs: Any) -> None:
3434
monkeypatch.undo()
3535

3636

37-
def test_no_timeout_passes_none_to_impit_sync(patch_request: list) -> None:
38-
"""Test that `'no_timeout'` passes `timeout=None` to impit (uses client-level default)."""
37+
def test_no_timeout_passes_large_value_to_impit_sync(patch_request: list) -> None:
38+
"""Test that `no_timeout` passes a large timeout to impit to effectively disable the timeout."""
3939
client = ImpitHttpClient(timeout_short=timedelta(seconds=10))
4040

4141
with pytest.raises(EndOfTestError):
4242
client.call(method='GET', url='http://placeholder.url/no_timeout', timeout='no_timeout')
4343

44-
assert patch_request == [None]
44+
assert patch_request == [86_400]
4545

4646

47-
async def test_no_timeout_passes_none_to_impit_async(patch_request: list) -> None:
48-
"""Test that `'no_timeout'` passes `timeout=None` to impit (uses client-level default)."""
47+
async def test_no_timeout_passes_large_value_to_impit_async(patch_request: list) -> None:
48+
"""Test that `no_timeout` passes a large timeout to impit to effectively disable the timeout."""
4949
client = ImpitHttpClientAsync(timeout_short=timedelta(seconds=10))
5050

5151
with pytest.raises(EndOfTestError):
5252
await client.call(method='GET', url='http://placeholder.url/no_timeout', timeout='no_timeout')
5353

54-
assert patch_request == [None]
54+
assert patch_request == [86_400]
5555

5656

5757
def test_default_timeout_uses_medium_tier_sync(patch_request: list) -> None:

0 commit comments

Comments
 (0)