Open
Conversation
github-actions
bot
added
the
t-tooling
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #604 +/- ##
===========================================
+ Coverage 76.02% 96.32% +20.29%
===========================================
Files 42 43 +1
Lines 2482 4274 +1792
===========================================
+ Hits 1887 4117 +2230
+ Misses 595 157 -438
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:
|
Pijukatel
reviewed
Feb 9, 2026
Pijukatel
reviewed
Feb 9, 2026
Pijukatel
reviewed
Feb 9, 2026
Mantisus
suggested changes
Feb 9, 2026
Pijukatel
reviewed
Feb 10, 2026
There was a problem hiding this comment.
Pull request overview
Major refactor that replaces dict-based API responses with generated, fully typed Pydantic models across the client, plus a new impit-based HTTP layer.
Changes:
- Introduces new typed resource clients and response wrappers around generated Pydantic models.
- Replaces legacy HTTP client with new
SyncHttpClient/AsyncHttpClientand updates tests/docs accordingly. - Adds extensive new unit/integration coverage and model generation tooling configuration.
Reviewed changes
Copilot reviewed 100 out of 106 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_client_timeouts.py | Updates timeout tests to use new http clients + timedelta. |
| tests/unit/test_client_request_queue.py | Updates request-queue tests for typed responses and explicit API URLs. |
| tests/unit/test_client_headers.py | Removes legacy header tests (likely superseded by new HTTP layer). |
| tests/unit/test_client_errors.py | Switches error tests to new http clients and improves assertion naming. |
| tests/unit/test_actor_start_params.py | Adds unit tests ensuring timeout query param is used with typed client. |
| tests/unit/conftest.py | Removes patch_basic_url fixture; keeps httpserver fixture. |
| tests/unit/README.md | Documents unit-test isolation requirements. |
| tests/integration/test_webhook_dispatch.py | Adds unified sync/async integration coverage for webhook dispatch endpoints. |
| tests/integration/test_webhook.py | Adds unified sync/async integration coverage for webhooks. |
| tests/integration/test_user.py | Adds unified sync/async integration coverage for user endpoints (typed). |
| tests/integration/test_store.py | Reworks store tests into unified sync/async style with typed models. |
| tests/integration/test_schedule.py | Adds unified sync/async integration coverage for schedules. |
| tests/integration/test_run_collection.py | Removes legacy run-collection integration tests (dict-based). |
| tests/integration/test_log.py | Adds unified sync/async integration coverage for logs. |
| tests/integration/test_build.py | Adds unified sync/async integration coverage for builds. |
| tests/integration/test_basic.py | Removes legacy basic integration tests (dict-based). |
| tests/integration/test_apify_client.py | Adds unified basic integration test using typed models. |
| tests/integration/test_actor_version.py | Adds unified sync/async integration coverage for actor versions. |
| tests/integration/test_actor_env_var.py | Adds unified sync/async integration coverage for actor env vars. |
| tests/integration/test_actor.py | Adds unified sync/async integration coverage for actors. |
| tests/integration/integration_test_utils.py | Removes old integration utilities (now likely in conftest). |
| tests/integration/README.md | Documents integration tests’ requirement for real API tokens. |
| src/apify_client/errors.py | Improves error docstrings and hardens JSON error extraction. |
| src/apify_client/clients/resource_clients/run_collection.py | Removes legacy client implementation (dict-based). |
| src/apify_client/clients/resource_clients/build.py | Removes legacy build client implementation (dict-based). |
| src/apify_client/clients/resource_clients/actor_env_var_collection.py | Removes legacy env var collection client (dict-based). |
| src/apify_client/clients/base/resource_collection_client.py | Removes legacy base list/create abstraction (dict-based). |
| src/apify_client/clients/base/resource_client.py | Removes legacy base resource client abstraction (dict-based). |
| src/apify_client/clients/base/base_client.py | Removes legacy base client (replaced by new resource client base). |
| src/apify_client/clients/base/actor_job_base_client.py | Removes legacy polling/abort base (replaced elsewhere). |
| src/apify_client/clients/base/init.py | Removes legacy base exports. |
| src/apify_client/clients/init.py | Removes legacy large re-export module. |
| src/apify_client/_types.py | Removes old JSON/ListPage types (moved/replaced). |
| src/apify_client/_statistics.py | Renames Statistics -> ClientStatistics. |
| src/apify_client/_resource_clients/webhook_dispatch_collection.py | Converts to typed models + new base resource client. |
| src/apify_client/_resource_clients/webhook_dispatch.py | Converts get() to typed models and explicit 404 handling. |
| src/apify_client/_resource_clients/webhook_collection.py | Converts list/create to typed models and new representation helpers. |
| src/apify_client/_resource_clients/webhook.py | Converts get/update/delete/test/dispatches to typed models and new base. |
| src/apify_client/_resource_clients/user.py | Converts user endpoints to typed models and new base. |
| src/apify_client/_resource_clients/store_collection.py | Converts store listing to typed models and new base. |
| src/apify_client/_resource_clients/schedule_collection.py | Converts schedule list/create to typed models and new base. |
| src/apify_client/_resource_clients/schedule.py | Converts schedule get/update/delete/log to typed models and new base. |
| src/apify_client/_resource_clients/run_collection.py | Adds new typed run collection client with pagination iteration. |
| src/apify_client/_resource_clients/request_queue_collection.py | Converts list/get_or_create to typed models and new base. |
| src/apify_client/_resource_clients/log.py | Updates to new base client, typed run usage, and task lifecycle handling. |
| src/apify_client/_resource_clients/key_value_store_collection.py | Converts list/get_or_create to typed models and new base. |
| src/apify_client/_resource_clients/dataset_collection.py | Converts list/get_or_create to typed models and new base. |
| src/apify_client/_resource_clients/build_collection.py | Converts list to typed models and new base. |
| src/apify_client/_resource_clients/build.py | Adds new typed build client with log + wait_for_finish. |
| src/apify_client/_resource_clients/actor_version_collection.py | Converts list/create to typed models and new base. |
| src/apify_client/_resource_clients/actor_env_var_collection.py | Adds new typed actor env var collection client. |
| src/apify_client/_resource_clients/actor_env_var.py | Converts env var get/update/delete to typed models and new base. |
| src/apify_client/_resource_clients/init.py | Expands exports for log streaming/status watchers; updates resource exports. |
| src/apify_client/_logging.py | Refactors logging helpers, adds redirect logger utilities, updates context injection. |
| src/apify_client/_internal_models.py | Adds internal minimal Pydantic models for polling/validation (non-generated). |
| src/apify_client/_http_clients/_sync.py | Adds new synchronous impit-based HTTP client with retries/backoff. |
| src/apify_client/_http_clients/_base.py | Adds shared HTTP base: headers, param conversion, gzip JSON, timeout scaling. |
| src/apify_client/_http_clients/_async.py | Adds new async impit-based HTTP client with retries/backoff. |
| src/apify_client/_http_clients/init.py | Adds package entrypoint for new HTTP clients. |
| src/apify_client/_http_client.py | Removes legacy HTTP client implementation. |
| src/apify_client/_consts.py | Adds shared constants (timeouts, retries, terminal statuses, API URL). |
| src/apify_client/_client_registry.py | Adds sync/async registries for DI and avoiding circular imports. |
| src/apify_client/init.py | Switches public import to new _apify_client module. |
| scripts/utils.py | Improves docstring conversion and typing in scripts. |
| scripts/fix_async_docstrings.py | Skips _http_clients and handles missing sync class/methods. |
| scripts/check_async_docstrings.py | Skips _http_clients and handles missing sync classes. |
| pyproject.toml | Updates dependencies, adds datamodel-codegen config, fixes ruff ignore for generated models, bumps test concurrency, adds generate-models task. |
| docs/03_examples/code/03_retrieve_sync.py | Updates examples to attribute access for typed models. |
| docs/03_examples/code/03_retrieve_async.py | Updates examples to attribute access for typed models. |
| docs/03_examples/code/02_tasks_sync.py | Updates examples to typed Task/Run + attribute access. |
| docs/03_examples/code/02_tasks_async.py | Updates examples to typed Task/Run + attribute access. |
| docs/03_examples/code/01_input_sync.py | Updates timeout usage to timedelta. |
| docs/03_examples/code/01_input_async.py | Updates timeout usage to timedelta. |
| docs/02_concepts/code/05_retries_sync.py | Updates retry/timeout params to timedelta. |
| docs/02_concepts/code/05_retries_async.py | Updates retry/timeout params to timedelta. |
| docs/02_concepts/code/01_async_support.py | Updates example to attribute access for typed run id. |
| docs/01_overview/code/01_usage_sync.py | Updates example to attribute access for typed dataset id. |
| docs/01_overview/code/01_usage_async.py | Updates example to attribute access for typed dataset id. |
| .github/workflows/_tests.yaml | Increases CI test concurrency default to 16. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 tasks
vdusek
force-pushed
the
generated-typed-clients
branch
from
a745fa3 to
10fb739
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 101 out of 103 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/apify_client/_logging.py:1
- In async streaming, checking
self._streaming_task.done()before raising RuntimeError prevents detecting when a task exists but has already completed. If a completed task exists and start() is called again, it will create a new task, potentially leading to resource leaks if the old task wasn't properly cleaned up. The condition should either always reject ifself._streaming_taskexists, or should setself._streaming_task = Noneafter completion.
from __future__ import annotations
src/apify_client/_logging.py:1
- Same issue as the streaming task - checking
self._logging_task.done()before raising RuntimeError means a completed logging task won't prevent start() from being called again, potentially creating resource leaks. The condition should either always reject ifself._logging_taskexists, or should setself._logging_task = Noneafter completion.
from __future__ import annotations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = await maybe_await(second_build_client.abort()) | ||
| aborted_build = cast('Build', result) | ||
| assert aborted_build is not None | ||
| assert aborted_build.status.value in ('SUCCEEDED', 'FAILED', 'ABORTED') |
There was a problem hiding this comment.
This assertion allows 'ABORTED' as a valid status, but the comment on line 188 says "Test abort on already finished build". If the build is already finished with SUCCEEDED or FAILED status, calling abort() should not change it to ABORTED. The assertion should only check for SUCCEEDED or FAILED to match the test's intent, or the comment should be updated to clarify that ABORTED is also acceptable.
Suggested change
| assert aborted_build.status.value in ('SUCCEEDED', 'FAILED', 'ABORTED') | |
| assert aborted_build.status.value in ('SUCCEEDED', 'FAILED') |
| try: | ||
| response_data = response.json() | ||
| if 'error' in response_data: | ||
| if isinstance(response_data, dict) and 'error' in response_data: |
There was a problem hiding this comment.
The type check isinstance(response_data, dict) is added, but the subsequent access response_data['error']['message'] and response_data['error']['type'] still assumes nested dict structure without validation. Consider also checking that response_data['error'] is a dict before accessing its keys to prevent potential KeyError or TypeError.
| def set_current_package_version(version: str) -> None: | ||
| with open(PYPROJECT_TOML_FILE_PATH, 'r+', encoding='utf-8') as pyproject_toml_file: | ||
| updated_pyproject_toml_file_lines = [] | ||
| updated_pyproject_toml_file_lines = list[str]() |
There was a problem hiding this comment.
This creates a type object list[str], not an empty list instance. This should be updated_pyproject_toml_file_lines = [] or updated_pyproject_toml_file_lines: list[str] = []. The code will fail when trying to call .append() on this type object.
Suggested change
| updated_pyproject_toml_file_lines = list[str]() | |
| updated_pyproject_toml_file_lines: list[str] = [] |
| @@ -17,8 +17,6 @@ | |||
|
|
|||
| # Find all classes which end with "ClientAsync" (there should be at most 1 per file) | |||
| async_class = red.find('ClassNode', name=re.compile('.*ClientAsync$')) | |||
There was a problem hiding this comment.
After removing the check for async_class existence (line 20-21 in the old code), the code now assumes async_class is never None. If no async class is found in a file, this will cause AttributeError when accessing async_class.name on line 22. The check should be restored, or a comment should explain why it's guaranteed to exist.
Suggested change
| async_class = red.find('ClassNode', name=re.compile('.*ClientAsync$')) | |
| async_class = red.find('ClassNode', name=re.compile('.*ClientAsync$')) | |
| if async_class is None: | |
| # No async client class in this file, nothing to fix | |
| continue |
Contributor
Author
|
Everything has been addressed. Feel free to add more feedback or reopen any conversations if you feel something wasn't properly resolved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is a major refactoring that introduces fully typed Pydantic models throughout the client library. The models are generated from the OpenAPI specifications. All API responses now return typed objects instead of raw dictionaries.
This follows up on apify/apify-docs#2182.
Issues
Packages
Pydantic.apify-shared.Key changes
pyproject.tomlto generate Pydantic models based on the OpenAPI specs.Actor,Task,Run, etc.).apify/apify-sdk-pythonfor details - chore: Adapt to apify-client v3 [WIP] apify-sdk-python#719.Architecture
ClientRegistryto be able to achieve that (because of resource clients-siblings imports).Breaking changes
result['key']) to attribute-style (result.key).Test plan
Next steps