feat!: Introduce tiered timeout system with per-endpoint configuration#653
feat!: Introduce tiered timeout system with per-endpoint configuration#653
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #653 +/- ##
==========================================
+ Coverage 96.45% 96.79% +0.34%
==========================================
Files 45 45
Lines 4318 4340 +22
==========================================
+ Hits 4165 4201 +36
+ Misses 153 139 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9653d95 to
634577c
Compare
634577c to
ac20367
Compare
Pijukatel
left a comment
There was a problem hiding this comment.
I would be very careful about changing the actual timeouts. Can we please split this into 2 PRs:
- Refactoring (majority)
- Assigning different timeouts to API operations
5071549 to
3d9f230
Compare
3d9f230 to
0ad2c46
Compare
There was a problem hiding this comment.
Pull request overview
Introduces a tiered timeout system across the Apify Python client, enabling per-endpoint timeout configuration via a shared Timeout type and centralized timeout resolution in HTTP clients.
Changes:
- Added timeout tiers (
short/medium/long+no_timeout) and propagated atimeoutparameter through most resource-client methods. - Updated HTTP client internals to resolve timeout tiers and apply exponential per-attempt timeout growth capped by
timeout_max. - Updated tests/docs for the new timeout API and reduced default retries (
DEFAULT_MAX_RETRIES8 → 4).
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_pluggable_http_client.py | Updates fake custom HTTP clients and tests to use Timeout and pass per-call timeout tiers. |
| tests/unit/test_http_clients.py | Adjusts base HTTP client init test to the new tiered timeout fields. |
| tests/unit/test_client_timeouts.py | Adds unit coverage for tier resolution, no_timeout, and timeout growth/capping. |
| tests/unit/test_actor_start_params.py | Updates Actor start tests to use run_timeout instead of timeout. |
| tests/integration/test_webhook.py | Fixes integration test type expectation for Actor call result. |
| src/apify_client/_types.py | Introduces Timeout and consolidates JsonSerializable plus minimal polling models. |
| src/apify_client/_resource_clients/webhook_dispatch_collection.py | Adds timeout parameter to list operations and forwards to _list. |
| src/apify_client/_resource_clients/webhook_dispatch.py | Adds timeout parameter to get operations and forwards to _get. |
| src/apify_client/_resource_clients/webhook_collection.py | Adds timeout parameter to list/create and forwards to _list/_create. |
| src/apify_client/_resource_clients/webhook.py | Adds timeout parameter to CRUD/test methods and forwards to HTTP calls. |
| src/apify_client/_resource_clients/user.py | Adds timeout parameter to user endpoints and forwards to _get/_http_client.call. |
| src/apify_client/_resource_clients/task_collection.py | Adds request timeout and renames run timeout → run_timeout for create. |
| src/apify_client/_resource_clients/task.py | Adds request timeout broadly; renames run timeout → run_timeout for start/call/update. |
| src/apify_client/_resource_clients/store_collection.py | Adds timeout parameter to list and forwards to _list. |
| src/apify_client/_resource_clients/schedule_collection.py | Adds timeout parameter to list/create and forwards to _list/_create. |
| src/apify_client/_resource_clients/schedule.py | Adds timeout parameter to schedule operations and forwards to _get/_update/_delete. |
| src/apify_client/_resource_clients/run_collection.py | Adds timeout parameter to run listing and forwards to _list. |
| src/apify_client/_resource_clients/run.py | Adds request timeout widely; renames run timeout → run_timeout where applicable; defaults waiters to no_timeout. |
| src/apify_client/_resource_clients/request_queue_collection.py | Adds timeout parameter to list/get_or_create and forwards to _list/_get_or_create. |
| src/apify_client/_resource_clients/request_queue.py | Replaces old fixed constants with tiered timeout parameters for queue operations. |
| src/apify_client/_resource_clients/log.py | Adds timeout parameter to log get/stream methods and forwards to HTTP calls. |
| src/apify_client/_resource_clients/key_value_store_collection.py | Adds timeout parameter to list/get_or_create and forwards to _list/_get_or_create. |
| src/apify_client/_resource_clients/key_value_store.py | Replaces old fixed constants with tiered timeout parameters across KVS operations. |
| src/apify_client/_resource_clients/dataset_collection.py | Adds timeout parameter to list/get_or_create and forwards to _list/_get_or_create. |
| src/apify_client/_resource_clients/dataset.py | Replaces old fixed constants with tiered timeout parameters across dataset operations. |
| src/apify_client/_resource_clients/build_collection.py | Adds timeout parameter to list and forwards to _list. |
| src/apify_client/_resource_clients/build.py | Adds timeout parameter to build operations and forwards to _get/_delete/_wait_for_finish. |
| src/apify_client/_resource_clients/actor_version_collection.py | Adds timeout parameter to list/create and forwards to _list/_create. |
| src/apify_client/_resource_clients/actor_version.py | Adds timeout parameter to get/update/delete and forwards to _get/_update/_delete. |
| src/apify_client/_resource_clients/actor_env_var_collection.py | Adds timeout parameter to list/create and forwards to _list/_create. |
| src/apify_client/_resource_clients/actor_env_var.py | Adds timeout parameter to get/update/delete and forwards to _get/_update/_delete. |
| src/apify_client/_resource_clients/actor_collection.py | Adds timeout parameter to list/create and forwards to _list/_create. |
| src/apify_client/_resource_clients/actor.py | Adds request timeout widely; renames run timeout → run_timeout for start/call. |
| src/apify_client/_resource_clients/_resource_client.py | Makes internal CRUD/list/create helpers require timeout and forwards it to HTTP client calls. |
| src/apify_client/_http_clients/_impit.py | Updates Impit clients to use tiered timeouts and per-attempt computed timeouts. |
| src/apify_client/_http_clients/_base.py | Implements _compute_timeout() tier resolution + exponential growth capped by timeout_max; updates call signatures to Timeout. |
| src/apify_client/_consts.py | Replaces single default timeout + old operation constants with tier defaults and DEFAULT_TIMEOUT_MAX; lowers DEFAULT_MAX_RETRIES. |
| src/apify_client/_apify_client.py | Changes client constructors to accept tiered timeouts and passes them into default HTTP clients. |
| src/apify_client/init.py | Exports Timeout from the package top-level. |
| docs/02_concepts/code/05_retries_sync.py | Updates docs example to use new retry/timeout configuration arguments. |
| docs/02_concepts/code/05_retries_async.py | Updates docs example to use new retry/timeout configuration arguments. |
Comments suppressed due to low confidence (1)
src/apify_client/_types.py:20
- The
JsonSerializabledocstring referencesjson.parse, which doesn’t exist in Python’s stdlib (json.loads/json.dumpsare the relevant APIs). Consider correcting the reference and removing the anecdotal claim about approval, so the type alias docs stay accurate and focused.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pijukatel
left a comment
There was a problem hiding this comment.
I think it is good if we want to expose the timeouts on the API client method level as well. But are there any usecases for it?
Users already have the option to change to some extent the timeout behavior by giving their own HTTP client - like client that inherits from ImpitHttpClient and just modifies the timeout-based behavior.
|
@janbuchar could you please take a look, as such refactoring will probably make its way to JS version as well. It would be for the best to comment on any changes now. |
9709a34 to
7bf700a
Compare
7bf700a to
6a22ea5
Compare
|
FYI; I rebased and consolidated commits - the 6a22ea5 is the state @Pijukatel reviewed. I will address his and the copilot's comments in the following commit. |
|
All comments addressed. Updates of tiers and default values will follow - we decided to split the refactor itself from the assignment of tiers and defaults. Also, once apify/impit#401 is implemented and released, we can remove the Impit I'll document the breaking changes (together with the others) in the upgrade guide in a separate PR. We can also add a commit to the release-v2 maintenance branch to mark the future deprecation of the timeout argument (and others if there are more). |
a810272 to
cfc0741
Compare
cfc0741 to
a810272
Compare
Summary
short(5s by default),medium(30s by default),long(360s by default),max(360s cap by default).Timeouttype alias:timedelta | Literal['no_timeout', 'short', 'medium', 'long']timeout_max)._internal_models.pyto_types.pyand consolidate type aliases (Timeout,JsonSerializable) into it.FAST_OPERATION_TIMEOUT,STANDARD_OPERATION_TIMEOUT).Timeout tiers
shortmediumlongno_timeoutmaxLet's discuss the best defaults later in a dedicated PR.
What this solves
Test plan