improve update class instantiation error handling (#2085)

CHANGELOG.md

@@ -21,6 +21,12 @@ Changes are grouped as follows
 ### Added
 - Support for the `/simulators` and `/simulators/integration` API endpoints.
 
+## [7.71.4] - 2025-01-09
+### Changed
+- Update classes accepting insance_id now raise when id/external_id are also given.
+### Added
+- All update classes warn when external_id is ignored (when id is also given, it takes precedence)
+
 ## [7.71.3] - 2025-01-09
 ### Added
 - `ResultSetExpression` now support the `skip_already_deleted` flag.

cognite/client/_version.py

@@ -1,5 +1,5 @@
 from __future__ import annotations
 
-__version__ = "7.71.3"
+__version__ = "7.71.4"
 
 __api_subversion__ = "20230101"

cognite/client/data_classes/_base.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import warnings
 from abc import ABC, abstractmethod
 from collections import UserList
 from collections.abc import Collection, Iterable, Iterator, Sequence
@@ -457,6 +458,13 @@ def __init__(self, id: int | None = None, external_id: str | None = None) -> Non
         self._external_id = external_id
         self._update_object: dict[str, Any] = {}
 
+        if id is not None and external_id is not None:
+            warnings.warn(
+                "Update object got both of 'id' and 'external_id', 'external_id' will be ignored.",
+                UserWarning,
+                stacklevel=2,
+            )
+
     def __eq__(self, other: Any) -> bool:
         return type(self) is type(other) and self.dump() == other.dump()
 

cognite/client/data_classes/files.py

@@ -391,6 +391,9 @@ class FileMetadataUpdate(CogniteUpdate):
     def __init__(
         self, id: int | None = None, external_id: str | None = None, instance_id: NodeId | None = None
     ) -> None:
+        if instance_id is not None and (id is not None or external_id is not None):
+            raise ValueError("Exactly one of 'id', 'external_id' or 'instance_id' must be provided.")
+
         super().__init__(id=id, external_id=external_id)
         self.instance_id = instance_id
 

cognite/client/data_classes/time_series.py

@@ -349,6 +349,9 @@ class TimeSeriesUpdate(CogniteUpdate):
     def __init__(
         self, id: int | None = None, external_id: str | None = None, instance_id: NodeId | None = None
     ) -> None:
+        if instance_id is not None and (id is not None or external_id is not None):
+            raise ValueError("Exactly one of 'id', 'external_id' or 'instance_id' must be provided.")
+
         super().__init__(id=id, external_id=external_id)
         self.instance_id = instance_id
 

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "cognite-sdk"
 
-version = "7.71.3"
+version = "7.71.4"
 
 description = "Cognite Python SDK"
 readme = "README.md"

tests/tests_unit/test_api/test_files.py

@@ -20,6 +20,7 @@
     LabelFilter,
     TimestampRange,
 )
+from cognite.client.data_classes.data_modeling.ids import NodeId
 from cognite.client.exceptions import CogniteAPIError, CogniteAuthorizationError
 from tests.utils import jsgz_load, set_request_limit
 
@@ -364,6 +365,20 @@ def test_update_with_update_class(self, cognite_client, mock_files_response):
             mock_files_response.calls[0].request.body
         )
 
+    def test_update_with_update_class_using_instance_id(self, cognite_client, mock_files_response):
+        res = cognite_client.files.update(FileMetadataUpdate(instance_id=NodeId("foo", "bar")).source.set("bla"))
+        assert isinstance(res, FileMetadata)
+        assert {
+            "items": [{"instanceId": {"space": "foo", "externalId": "bar"}, "update": {"source": {"set": "bla"}}}]
+        } == jsgz_load(mock_files_response.calls[0].request.body)
+
+    @pytest.mark.parametrize("extra_identifiers", (dict(id=1), dict(external_id="a"), dict(id=1, external_id="a")))
+    def test_update_with_update_class_using_instance_id_and_other_identifier(
+        self, extra_identifiers, cognite_client, mock_files_response
+    ):
+        with pytest.raises(ValueError, match="Exactly one of 'id', 'external_id' or 'instance_id' must be provided."):
+            FileMetadataUpdate(instance_id=NodeId("foo", "bar"), **extra_identifiers)
+
     def test_update_labels_single(self, cognite_client, mock_files_response):
         cognite_client.files.update([FileMetadataUpdate(id=1).labels.add("PUMP").labels.remove("WELL LOG")])
         expected = {"labels": {"add": [{"externalId": "PUMP"}], "remove": [{"externalId": "WELL LOG"}]}}

tests/tests_unit/test_base.py

@@ -700,7 +700,8 @@ def test_dump_external_id(self):
         assert {"externalId": "1", "update": {}} == MyUpdate(external_id="1").dump()
 
     def test_dump_both_ids_set(self):
-        assert {"id": 1, "update": {}} == MyUpdate(id=1, external_id="1").dump()
+        with pytest.warns(UserWarning, match="'external_id' will be ignored"):
+            assert {"id": 1, "update": {}} == MyUpdate(id=1, external_id="1").dump()
 
     def test_eq(self):
         assert MyUpdate() == MyUpdate()