Improve tests for update modes and fix bug in replace mode (#1936)

cognitedata · Sep 18, 2024 · 6bd3ca7 · 6bd3ca7
1 parent b79f4b6
commit 6bd3ca7
Show file tree

Hide file tree

Showing 25 changed files with 119 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,13 +17,17 @@ Changes are grouped as follows
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
+## [7.60.6] - 2024-09-17
+### Fixed
+- Fixed bug in `replace` upsert mode which caused objects to not be cleared.
+
 ## [7.60.5] - 2024-09-17
 ### Changed
-Remove beta notice on the Data Workflows `WorkflowTriggerAPI`
+- Remove beta notice on the Data Workflows `WorkflowTriggerAPI`
 
 ## [7.60.4] - 2024-09-15
 ### Added
-Fix bug in column name remapping for `TypedInstance.to_pandas()`
+- Fix bug in column name remapping for `TypedInstance.to_pandas()`
 
 ## [7.60.3] - 2024-09-14
 ### Changed

diff --git a/cognite/client/_api/functions.py b/cognite/client/_api/functions.py
@@ -350,7 +350,7 @@ def _create_function_obj(
         assert_type(cpu, "cpu", [float], allow_none=True)
         assert_type(memory, "memory", [float], allow_none=True)
         sleep_time = 1.0  # seconds
-        for i in range(5):
+        for i in range(MAX_RETRIES):
             file = self._cognite_client.files.retrieve(id=file_id)
             if file and file.uploaded:
                 break

diff --git a/cognite/client/_api_client.py b/cognite/client/_api_client.py
@@ -1246,7 +1246,7 @@ def _convert_resource_to_patch_object(
             if (snake := to_snake_case(key)) not in update_attribute_by_name:
                 continue
             prop = update_attribute_by_name[snake]
-            if prop.is_container and mode == "patch":
+            if (prop.is_list or prop.is_object) and mode == "patch":
                 update[key] = {"add": value}
             else:
                 update[key] = {"set": value}
@@ -1255,14 +1255,21 @@ def _convert_resource_to_patch_object(
         return patch_object
 
     @staticmethod
-    def _clear_all_attributes(
-        update_attributes: list[PropertySpec],
-    ) -> dict[str, dict]:
-        return {
-            to_camel_case(prop.name): {"set": []} if prop.is_container else {"setNull": True}
-            for prop in update_attributes
-            if prop.is_nullable and not prop.is_beta
-        }
+    def _clear_all_attributes(update_attributes: list[PropertySpec]) -> dict[str, dict]:
+        cleared = {}
+        for prop in update_attributes:
+            if prop.is_beta:
+                continue
+            elif prop.is_object:
+                clear_with: dict = {"set": {}}
+            elif prop.is_list:
+                clear_with = {"set": []}
+            elif prop.is_nullable:
+                clear_with = {"setNull": True}
+            else:
+                continue
+            cleared[to_camel_case(prop.name)] = clear_with
+        return cleared
 
     @staticmethod
     def _status_ok(status_code: int) -> bool:

diff --git a/cognite/client/_version.py b/cognite/client/_version.py
@@ -1,4 +1,4 @@
 from __future__ import annotations
 
-__version__ = "7.60.5"
+__version__ = "7.60.6"
 __api_subversion__ = "20230101"
diff --git a/cognite/client/data_classes/_base.py b/cognite/client/data_classes/_base.py
@@ -434,11 +434,15 @@ def as_write(self) -> CogniteResourceList[T_WriteClass]:
 @dataclass
 class PropertySpec:
     name: str
-    is_container: bool = False
+    is_list: bool = False
+    is_object: bool = False
     is_nullable: bool = True
     # Used to skip replace when the value is None
     is_beta: bool = False
 
+    def __post_init__(self) -> None:
+        assert not (self.is_list and self.is_object), "PropertySpec cannot be both list and object"
+
 
 class CogniteUpdate:
     def __init__(self, id: int | None = None, external_id: str | None = None) -> None:

diff --git a/cognite/client/data_classes/assets.py b/cognite/client/data_classes/assets.py
@@ -523,11 +523,11 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("name", is_nullable=False),
             PropertySpec("description"),
             PropertySpec("data_set_id"),
-            PropertySpec("metadata", is_container=True),
+            PropertySpec("metadata", is_object=True),
             PropertySpec("source"),
             PropertySpec("parent_id", is_nullable=False),
             PropertySpec("parent_external_id", is_nullable=False),
-            PropertySpec("labels", is_container=True),
+            PropertySpec("labels", is_list=True),
             PropertySpec("geo_location"),
         ]
 

diff --git a/cognite/client/data_classes/data_sets.py b/cognite/client/data_classes/data_sets.py
@@ -238,7 +238,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("external_id", is_nullable=False),
             PropertySpec("name"),
             PropertySpec("description"),
-            PropertySpec("metadata", is_container=True),
+            PropertySpec("metadata", is_object=True),
             PropertySpec("write_protected", is_nullable=False),
         ]
 

diff --git a/cognite/client/data_classes/datapoints_subscriptions.py b/cognite/client/data_classes/datapoints_subscriptions.py
@@ -217,7 +217,7 @@ def filter(self) -> _FilterDataPointSubscriptionUpdate:
     def _get_update_properties(cls, item: CogniteResource | None = None) -> list[PropertySpec]:
         return [
             PropertySpec("name"),
-            PropertySpec("time_series_ids", is_container=True),
+            PropertySpec("time_series_ids", is_list=True),
             PropertySpec("filter", is_nullable=False),
             PropertySpec("data_set_id"),
         ]

diff --git a/cognite/client/data_classes/events.py b/cognite/client/data_classes/events.py
@@ -370,8 +370,8 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("start_time"),
             PropertySpec("end_time"),
             PropertySpec("description"),
-            PropertySpec("metadata", is_container=True),
-            PropertySpec("asset_ids", is_container=True),
+            PropertySpec("metadata", is_object=True),
+            PropertySpec("asset_ids", is_list=True),
             PropertySpec("source"),
             PropertySpec("type"),
             PropertySpec("subtype"),

diff --git a/cognite/client/data_classes/extractionpipelines.py b/cognite/client/data_classes/extractionpipelines.py
@@ -356,9 +356,9 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("description", is_nullable=False),
             PropertySpec("data_set_id", is_nullable=False),
             PropertySpec("schedule", is_nullable=False),
-            PropertySpec("raw_tables", is_container=True),
-            PropertySpec("contacts", is_container=True),
-            PropertySpec("metadata", is_container=True),
+            PropertySpec("raw_tables", is_list=True),
+            PropertySpec("contacts", is_list=True),
+            PropertySpec("metadata", is_object=True),
             PropertySpec("source", is_nullable=False),
             PropertySpec("documentation", is_nullable=False),
             # Not supported yet

diff --git a/cognite/client/data_classes/files.py b/cognite/client/data_classes/files.py
@@ -487,10 +487,10 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
                 # If Instance ID is set, the file was created in DMS. Then, it is
                 # limited which properties can be updated. (Only the ones that are not in DMS + security categories)
                 PropertySpec("external_id"),
-                PropertySpec("metadata", is_container=True),
-                PropertySpec("asset_ids", is_container=True),
+                PropertySpec("metadata", is_object=True),
+                PropertySpec("asset_ids", is_list=True),
                 PropertySpec("data_set_id"),
-                PropertySpec("labels", is_container=True),
+                PropertySpec("labels", is_list=True),
                 PropertySpec("geo_location"),
             ]
 
@@ -500,13 +500,13 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("directory"),
             PropertySpec("source"),
             PropertySpec("mime_type"),
-            PropertySpec("metadata", is_container=True),
-            PropertySpec("asset_ids", is_container=True),
+            PropertySpec("metadata", is_object=True),
+            PropertySpec("asset_ids", is_list=True),
             PropertySpec("source_created_time"),
             PropertySpec("source_modified_time"),
             PropertySpec("data_set_id"),
-            PropertySpec("security_categories", is_container=True),
-            PropertySpec("labels", is_container=True),
+            PropertySpec("security_categories", is_list=True),
+            PropertySpec("labels", is_list=True),
             PropertySpec("geo_location"),
         ]
 

diff --git a/cognite/client/data_classes/hosted_extractors/sources.py b/cognite/client/data_classes/hosted_extractors/sources.py
@@ -546,10 +546,10 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
         return [
             PropertySpec("host", is_nullable=False),
             PropertySpec("port", is_nullable=True),
-            PropertySpec("authentication", is_nullable=True, is_container=True),
+            PropertySpec("authentication", is_nullable=True, is_object=True),
             PropertySpec("useTls", is_nullable=False),
-            PropertySpec("ca_certificate", is_nullable=True, is_container=True),
-            PropertySpec("auth_certificate", is_nullable=True, is_container=True),
+            PropertySpec("ca_certificate", is_nullable=True, is_object=True),
+            PropertySpec("auth_certificate", is_nullable=True, is_object=True),
         ]
 
 

diff --git a/cognite/client/data_classes/relationships.py b/cognite/client/data_classes/relationships.py
@@ -411,7 +411,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("start_time"),
             PropertySpec("end_time"),
             PropertySpec("data_set_id"),
-            PropertySpec("labels", is_container=True),
+            PropertySpec("labels", is_list=True),
         ]
 
 

diff --git a/cognite/client/data_classes/sequences.py b/cognite/client/data_classes/sequences.py
@@ -484,7 +484,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("description"),
             PropertySpec("external_id", is_nullable=False),
             PropertySpec("name"),
-            PropertySpec("metadata", is_container=True),
+            PropertySpec("metadata", is_object=True),
         ]
 
 
@@ -586,7 +586,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("description"),
             PropertySpec("asset_id"),
             # Sequences do not support setting metadata to an empty array.
-            PropertySpec("metadata", is_container=True, is_nullable=False),
+            PropertySpec("metadata", is_object=True, is_nullable=False),
             PropertySpec("data_set_id"),
             # PropertySpec("columns", is_list=True),
         ]

diff --git a/cognite/client/data_classes/three_d.py b/cognite/client/data_classes/three_d.py
@@ -211,7 +211,7 @@ def data_set_id(self) -> _PrimitiveThreeDModelUpdate:
     def _get_update_properties(cls, item: CogniteResource | None = None) -> list[PropertySpec]:
         return [
             PropertySpec("name", is_nullable=False),
-            PropertySpec("metadata", is_container=True),
+            PropertySpec("metadata", is_object=True),
             PropertySpec("data_set_id", is_nullable=True),
         ]
 
@@ -471,7 +471,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("camera", is_nullable=False),
             PropertySpec("scale", is_nullable=False),
             PropertySpec("translation", is_nullable=False),
-            PropertySpec("metadata", is_container=True),
+            PropertySpec("metadata", is_object=True),
         ]
 
 

diff --git a/cognite/client/data_classes/time_series.py b/cognite/client/data_classes/time_series.py
@@ -437,7 +437,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
                 # If Instance ID is set, the time series was created in DMS. Then, it is
                 # limited which properties can be updated. (Only the ones that are not in DMS + security categories)
                 PropertySpec("external_id", is_nullable=False),
-                PropertySpec("metadata", is_container=True, is_nullable=False),
+                PropertySpec("metadata", is_object=True, is_nullable=False),
                 PropertySpec("asset_id"),
                 PropertySpec("data_set_id"),
             ]
@@ -447,13 +447,13 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
                 PropertySpec("external_id", is_nullable=False),
                 PropertySpec("name"),
                 # TimeSeries does not support setting metadata to an empty array.
-                PropertySpec("metadata", is_container=True, is_nullable=False),
+                PropertySpec("metadata", is_object=True, is_nullable=False),
                 PropertySpec("unit"),
                 PropertySpec("unit_external_id"),
                 PropertySpec("asset_id"),
                 PropertySpec("description"),
                 PropertySpec("is_step", is_nullable=False),
-                PropertySpec("security_categories", is_container=True),
+                PropertySpec("security_categories", is_list=True),
                 PropertySpec("data_set_id"),
             ]
 

diff --git a/cognite/client/data_classes/transformations/__init__.py b/cognite/client/data_classes/transformations/__init__.py
@@ -657,7 +657,7 @@ def _get_update_properties(cls, item: CogniteResource | None = None) -> list[Pro
             PropertySpec("is_public", is_nullable=False),
             PropertySpec("ignore_null_fields", is_nullable=False),
             PropertySpec("data_set_id"),
-            PropertySpec("tags", is_container=True),
+            PropertySpec("tags", is_list=True),
         ]
 
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "cognite-sdk"
 
-version = "7.60.5"
+version = "7.60.6"
 description = "Cognite Python SDK"
 readme = "README.md"
 documentation = "https://cognite-sdk-python.readthedocs-hosted.com"

diff --git a/tests/tests_integration/test_api/test_assets.py b/tests/tests_integration/test_api/test_assets.py
@@ -256,9 +256,9 @@ def test_filter_by_geo_location(self, cognite_client):
             cognite_client.assets.delete(id=a.id)
 
     def test_upsert_2_asset_one_preexisting(self, cognite_client: CogniteClient) -> None:
-        new_asset = Asset(external_id="test_upsert_2_asset_one_preexisting:new", name="my new asset")
+        new_asset = Asset(external_id="test_upsert_2_asset_one_preexisting:new" + random_string(5), name="my new asset")
         preexisting = Asset(
-            external_id="test_upsert_2_asset_one_preexisting:preexisting",
+            external_id="test_upsert_2_asset_one_preexisting:preexisting" + random_string(5),
             name="my preexisting asset",
         )
         preexisting_update = Asset.load(preexisting.dump(camel_case=True))

diff --git a/tests/tests_integration/test_api/test_data_workflows.py b/tests/tests_integration/test_api/test_data_workflows.py
@@ -25,6 +25,7 @@
     WorkflowVersionUpsert,
 )
 from cognite.client.exceptions import CogniteAPIError
+from cognite.client.utils._text import random_string
 
 
 @pytest.fixture
@@ -266,7 +267,7 @@ def workflow_scheduled_trigger(cognite_client: CogniteClient, add_multiply_workf
 class TestWorkflows:
     def test_upsert_delete(self, cognite_client: CogniteClient) -> None:
         workflow = WorkflowUpsert(
-            external_id="integration_test-test_create_delete",
+            external_id="integration_test-test_create_delete" + random_string(5),
             description="This is ephemeral workflow for testing purposes",
         )
         cognite_client.workflows.delete(workflow.external_id, ignore_unknown_ids=True)
@@ -305,16 +306,18 @@ def test_retrieve_non_existing_workflow(self, cognite_client: CogniteClient) ->
 class TestWorkflowVersions:
     def test_upsert_delete(self, cognite_client: CogniteClient) -> None:
         version = WorkflowVersionUpsert(
-            workflow_external_id="integration_test-workflow_versions-test_create_delete",
+            workflow_external_id="integration_test-workflow_versions-test_create_delete" + random_string(5),
             version="1",
             workflow_definition=WorkflowDefinitionUpsert(
                 tasks=[
                     WorkflowTask(
-                        external_id="integration_test-workflow_definitions-test_create_delete-subworkflow1",
+                        external_id="integration_test-workflow_definitions-test_create_delete-subworkflow1"
+                        + random_string(5),
                         parameters=SubworkflowTaskParameters(
                             tasks=[
                                 WorkflowTask(
-                                    external_id="integration_test-workflow_definitions-test_create_delete-task1",
+                                    external_id="integration_test-workflow_definitions-test_create_delete-task1"
+                                    + random_string(5),
                                     parameters=FunctionTaskParameters(
                                         external_id="integration_test-workflow_definitions-test_create_delete-task1-function",
                                         data={"a": 1, "b": 2},

diff --git a/tests/tests_integration/test_api/test_relationships.py b/tests/tests_integration/test_api/test_relationships.py
@@ -241,9 +241,9 @@ def test_compare_partitioned_gen_and_list(self, cognite_client, create_multiple_
         assert {a.external_id for a in res_generator} == {a.external_id for a in res_list}
 
     def test_upsert_2_relationships_one_preexisting(self, cognite_client: CogniteClient) -> None:
-        asset1 = Asset(external_id="test_upsert_2_asset_one_preexisting:asset1", name="asset1")
-        asset2 = Asset(external_id="test_upsert_2_asset_one_preexisting:asset2", name="asset2")
-        asset3 = Asset(external_id="test_upsert_2_asset_one_preexisting:asset3", name="asset3")
+        asset1 = Asset(external_id="test_upsert_2_asset_one_preexisting:asset1" + random_string(5), name="asset1")
+        asset2 = Asset(external_id="test_upsert_2_asset_one_preexisting:asset2" + random_string(5), name="asset2")
+        asset3 = Asset(external_id="test_upsert_2_asset_one_preexisting:asset3" + random_string(5), name="asset3")
 
         new_relationship = Relationship(
             external_id=f"test_upsert_2_relationships_one_preexisting:new:{random_string(5)}",

diff --git a/tests/tests_integration/test_api/test_sequences.py b/tests/tests_integration/test_api/test_sequences.py
@@ -16,6 +16,7 @@
 )
 from cognite.client.data_classes.sequences import SequenceProperty, SortableSequenceProperty
 from cognite.client.exceptions import CogniteNotFoundError
+from cognite.client.utils._text import random_string
 from tests.utils import set_request_limit
 
 
@@ -211,12 +212,12 @@ def test_get_new(self, cognite_client, new_seq):
 
     def test_upsert_2_sequence_one_preexisting(self, cognite_client: CogniteClient) -> None:
         new_sequence = Sequence(
-            external_id="test_upsert_2_sequence_one_preexisting:new",
+            external_id="test_upsert_2_sequence_one_preexisting:new" + random_string(5),
             name="my new sequence",
             columns=SequenceColumnList([SequenceColumn(external_id="col1", value_type="String")]),
         )
         preexisting = Sequence(
-            external_id="test_upsert_2_sequence_one_preexisting:preexisting",
+            external_id="test_upsert_2_sequence_one_preexisting:preexisting" + random_string(5),
             name="my preexisting sequence",
             columns=SequenceColumnList([SequenceColumn(external_id="col1", value_type="String")]),
         )

diff --git a/tests/tests_integration/test_api/test_time_series.py b/tests/tests_integration/test_api/test_time_series.py
@@ -8,6 +8,7 @@
 from cognite.client.data_classes.data_modeling import Space
 from cognite.client.data_classes.data_modeling.cdm.v1 import CogniteTimeSeriesApply
 from cognite.client.data_classes.time_series import TimeSeriesProperty
+from cognite.client.utils._text import random_string
 from cognite.client.utils._time import MAX_TIMESTAMP_MS, MIN_TIMESTAMP_MS
 from tests.utils import set_request_limit
 
@@ -176,10 +177,10 @@ def test_delete_with_nonexisting(self, cognite_client):
 
     def test_upsert_2_time_series_one_preexisting(self, cognite_client: CogniteClient) -> None:
         new_times_series = TimeSeries(
-            external_id="test_upsert_2_time_series_one_preexisting:new", name="my new time series"
+            external_id="test_upsert_2_time_series_one_preexisting:new" + random_string(5), name="my new time series"
         )
         preexisting = TimeSeries(
-            external_id="test_upsert_2_time_series_one_preexisting:preexisting",
+            external_id="test_upsert_2_time_series_one_preexisting:preexisting" + random_string(5),
             name="my preexisting time series",
         )
         preexisting_update = TimeSeries.load(preexisting.dump(camel_case=True))