feat(integrations): Implement Integrations API client (CRUD + sub-resources) - #2820
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the Integrations API and its sub-APIs (tasks, errors, config, and actions) to the Cognite Python SDK, including both async and auto-generated sync versions, testing support, and client registration. Feedback on the changes highlights the need to support dictionary inputs in the create methods of both IntegrationActionsAPI and IntegrationConfigAPI to maintain consistency with other SDK creation methods and prevent potential AttributeErrors.
| single_item = isinstance(action, ActionWrite) | ||
| items: list[ActionWrite] = [action] if isinstance(action, ActionWrite) else list(action) |
There was a problem hiding this comment.
The current implementation of create assumes that action is either an instance of ActionWrite or a sequence of ActionWrite instances. However, if a user passes a dict or a sequence of dicts (which is a common pattern in the Cognite SDK), isinstance(action, ActionWrite) will evaluate to False, causing the method to treat the single dict as a sequence of its keys, leading to an AttributeError when .dump() is called. To ensure consistency and robustness, we should explicitly check for sequences and support loading from dictionaries.
if isinstance(action, Sequence) and not isinstance(action, (str, dict)):
single_item = False
items = [ActionWrite._load(item) if isinstance(item, dict) else item for item in action]
else:
single_item = True
items = [ActionWrite._load(action) if isinstance(action, dict) else action]References
- Consistency: Follow established patterns across the codebase (e.g., supporting dictionary inputs for resource creation methods). (link)
| if isinstance(config, ConfigRevision): | ||
| config = config.as_write() |
There was a problem hiding this comment.
If the user passes a dict representing the configuration revision, calling config.dump() will raise an AttributeError because dictionaries do not have a dump method. To maintain consistency with other SDK creation methods, we should support dictionary inputs by loading them into ConfigRevisionWrite instances.
if isinstance(config, dict):
config = ConfigRevisionWrite._load(config)
elif isinstance(config, ConfigRevision):
config = config.as_write()References
- Consistency: Follow established patterns across the codebase (e.g., supporting dictionary inputs for resource creation methods). (link)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## integrations-part1-data-classes #2820 +/- ##
===================================================================
- Coverage 93.14% 93.10% -0.05%
===================================================================
Files 523 533 +10
Lines 54012 54330 +318
===================================================================
+ Hits 50311 50583 +272
- Misses 3701 3747 +46
🚀 New features to boost your workflow:
|
…ources) Adds IntegrationsAPI with core CRUD (list/create/retrieve/update/delete) and its Tasks, Errors, Config, and Actions sub-APIs, wired into CogniteClient/AsyncCogniteClient and testing.py. Uses the beta 'cdf-version' header via the new _beta_version_header() helper. Unit tests for this surface are added in the following PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
33f7b9a to
6fdd3ea
Compare
Summary
Adds IntegrationsAPI with core CRUD (list/create/retrieve/update/delete) and its Tasks, Errors, Config, and Actions sub-APIs, wired into CogniteClient/AsyncCogniteClient and testing.py. This is the PR where
client.integrationsbecomes usable. Builds on the data classes from #2819. Unit tests for this surface are added in #2821.Type of change
What changed
cognite/client/_api/integrations/:IntegrationsAPI(__init__.py) plusIntegrationTasksAPI,IntegrationErrorsAPI,IntegrationConfigAPI,IntegrationActionsAPI.IntegrationsAPI:list(),__call__(async iterator),create(),retrieve(),update(),delete()— all using the generic_list/_create_multiple/_retrieve_multiple/_update_multiple/_delete_multiplehelpers.IntegrationTasksAPI.list_history()/sync(),IntegrationErrorsAPI.list(),IntegrationConfigAPIcreate/retrieve/list revisions,IntegrationActionsAPIcreate/list/retrieve/cancel (manual chunking forcreate/cancelsince the integration's external_id is a query param, not part of the item body)._beta_version_header()onBasicAsyncAPIClient(_basic_api_client.py), symmetric to the existing_alpha_version_header(), producing thecdf-version: <sub>-betaheader sent on every call.self.integrations = IntegrationsAPI(...)intoAsyncCogniteClient(_cognite_client.py); sync mirror auto-generated into_sync_cognite_client.py/_sync_api/integrations/**.IntegrationsAPI+ all 4 sub-APIs inCogniteClientMock/AsyncCogniteClientMock(testing.py).integrations,integrations/config,integrations/actions,integrations/actions/cancelto the non-idempotent/non-retryable POST classification inutils/_url.py.Why it changed
SimulatorsAPI; wire schema fromcognitedata/service-contractsPR #3378; field-level details confirmed against the localodinservice implementation.What to focus on during review
IntegrationsAPI.__init__eagerly constructs all four sub-APIs (tasks/errors/config/actions) in the same constructor call — this is a real, hard coupling (also required bytesting.py's mock) and is why the client/sub-API split isn't finer-grained.api_maturity="beta",sdk_maturity="alpha"on theFeaturePreviewWarning— every call emits this warning and sends the betacdf-versionheader.IntegrationActionsAPI.create()/cancel()bypass the generic multi-item helpers because the integration's external_id is a query param, not part of the item body — worth double-checking the manual request construction there.IntegrationConfigAPI.list()is a single call, not_list/_list_generator— the revisions-list response has nonextCursordespite accepting acursorparam.test_meta.py,test_testing.py,test_cognite_client.py,test_api_client.py) passes unchanged.Test evidence
pytest tests/tests_unit/test_testing.py tests/tests_unit/test_meta.py tests/tests_unit/test_api_client.py tests/tests_unit/test_cognite_client.py -q→ 866 passedpython scripts/sync_client_codegen/main.py verify→ sync mirrors up to dateruff check/ruff format --check→ cleanmypy→ no issues (543 source files)Risks and unknowns
Rollout and rollback
FeaturePreviewWarning(api_maturity="beta", sdk_maturity="alpha")— users get a warning on first use, no opt-in flag needed. No migrations. Revert is a straight revert of this commit (and feat(integrations): Add Integrations API data classes #2819 if that's also being rolled back).Checklist