[BUGFIX] fix add_or_update_expectation_suite update path

great_expectations/data_context/data_context/abstract_data_context.py

@@ -2938,6 +2938,7 @@ def add_expectation_suite(
 
         Raises:
             DataContextError: A suite with the same name already exists (and `overwrite_existing` is not enabled).
+            ValueError: The arguments provided are invalid.
         """
         return self._add_expectation_suite(
             expectation_suite_name=expectation_suite_name,
@@ -2965,16 +2966,18 @@ def _add_expectation_suite(
         **kwargs,
     ) -> ExpectationSuite:
         if not isinstance(overwrite_existing, bool):
-            raise ValueError("Parameter overwrite_existing must be of type BOOL")
-        if not ((expectation_suite_name is None) ^ (expectation_suite is None)):
-            raise ValueError(
-                "Must either pass in an existing expectation_suite or individual constructor arguments (but not both)"
-            )
+            raise ValueError("overwrite_existing must be of type bool.")
+
+        self._validate_expectation_suite_xor_expectation_suite_name(
+            expectation_suite, expectation_suite_name
+        )
 
         if not expectation_suite:
-            assert (
-                expectation_suite_name
-            ), "If constructing a suite with individual args, suite name must be guaranteed"
+            # type narrowing
+            assert isinstance(
+                expectation_suite_name, str
+            ), "expectation_suite_name must be specified."
+
             expectation_suite = ExpectationSuite(
                 expectation_suite_name=expectation_suite_name,
                 data_context=self,
@@ -3031,6 +3034,15 @@ def update_expectation_suite(
         Raises:
             DataContextError: A suite with the given name does not already exist.
         """
+        return self._update_expectation_suite(expectation_suite=expectation_suite)
+
+    def _update_expectation_suite(
+        self,
+        expectation_suite: ExpectationSuite,
+    ) -> ExpectationSuite:
+        """
+        Like `update_expectation_suite` but without the usage statistics logging.
+        """
         name = expectation_suite.expectation_suite_name
         id = expectation_suite.ge_cloud_id
         key = self._determine_key_for_suite_update(name=name, id=id)
@@ -3102,30 +3114,52 @@ def add_or_update_expectation_suite(
         """Add a new ExpectationSuite or update an existing one on the context depending on whether it already exists or not.
 
         Args:
-            expectation_suite_name: The name of the suite to create.
-            id: Identifier to associate with this suite.
+            expectation_suite_name: The name of the suite to create or replace.
+            id: Identifier to associate with this suite (ignored if updating existing suite).
             expectations: Expectation Configurations to associate with this suite.
             evaluation_parameters: Evaluation parameters to be substituted when evaluating Expectations.
-            data_asset_type: Type of data asset to associate with this suite.
-            execution_engine_type: Name of the execution engine type.
+            data_asset_type: Type of Data Asset to associate with this suite.
+            execution_engine_type: Name of the Execution Engine type.
             meta: Metadata related to the suite.
-            expectation_suite: An existing ExpectationSuite object you wish to persist.
+            expectation_suite: The `ExpectationSuite` object you wish to persist.
 
         Returns:
-            A new ExpectationSuite or an updated once (depending on whether or not it existed before this method call).
+            The persisted `ExpectationSuite`.
         """
-        return self._add_expectation_suite(
-            expectation_suite_name=expectation_suite_name,
-            id=id,
-            expectations=expectations,
-            evaluation_parameters=evaluation_parameters,
-            data_asset_type=data_asset_type,
-            execution_engine_type=execution_engine_type,
-            meta=meta,
-            expectation_suite=expectation_suite,
-            overwrite_existing=True,  # `add_or_update` always overwrites.
+
+        self._validate_expectation_suite_xor_expectation_suite_name(
+            expectation_suite, expectation_suite_name
         )
 
+        if not expectation_suite:
+            # type narrowing
+            assert isinstance(
+                expectation_suite_name, str
+            ), "expectation_suite_name must be specified."
+
+            expectation_suite = ExpectationSuite(
+                expectation_suite_name=expectation_suite_name,
+                data_context=self,
+                ge_cloud_id=id,
+                expectations=expectations,
+                evaluation_parameters=evaluation_parameters,
+                data_asset_type=data_asset_type,
+                execution_engine_type=execution_engine_type,
+                meta=meta,
+            )
+
+        try:
+            existing = self.get_expectation_suite(
+                expectation_suite_name=expectation_suite.name
+            )
+        except gx_exceptions.DataContextError:
+            # not found
+            return self._add_expectation_suite(expectation_suite=expectation_suite)
+
+        # The suite object must have an ID in order to request a PUT to GX Cloud.
+        expectation_suite.ge_cloud_id = existing.ge_cloud_id
+        return self._update_expectation_suite(expectation_suite=expectation_suite)
+
     @public_api
     @new_argument(
         argument_name="id",
@@ -5607,3 +5641,23 @@ def _resolve_id_and_ge_cloud_id(
         if id and ge_cloud_id:
             raise ValueError("Please only pass in either id or ge_cloud_id (not both)")
         return id or ge_cloud_id
+
+    @staticmethod
+    def _validate_expectation_suite_xor_expectation_suite_name(
+        expectation_suite: Optional[ExpectationSuite] = None,
+        expectation_suite_name: Optional[str] = None,
+    ) -> None:
+        """
+        Validate that only one of expectation_suite or expectation_suite_name is specified.
+
+        Raises:
+            ValueError: Invalid arguments.
+        """
+        if expectation_suite_name is not None and expectation_suite is not None:
+            raise ValueError(
+                "Only one of expectation_suite_name or expectation_suite may be specified."
+            )
+        if expectation_suite_name is None and expectation_suite is None:
+            raise ValueError(
+                "One of expectation_suite_name or expectation_suite must be specified."
+            )

great_expectations/data_context/data_context/cloud_data_context.py

@@ -512,7 +512,7 @@ def create_expectation_suite(
             DeprecationWarning,
         )
         if not isinstance(overwrite_existing, bool):
-            raise ValueError("Parameter overwrite_existing must be of type BOOL")
+            raise ValueError("overwrite_existing must be of type bool.")
 
         expectation_suite = ExpectationSuite(
             expectation_suite_name=expectation_suite_name, data_context=self

tests/data_context/cloud_data_context/test_expectation_suite_crud.py

@@ -603,11 +603,15 @@ def test_add_or_update_expectation_suite_adds_new_obj(
         f"{GXCloudStoreBackend.__module__}.{GXCloudStoreBackend.__name__}.has_key",
         return_value=False,
     ), mock.patch(
-        f"{GXCloudStoreBackend.__module__}.{GXCloudStoreBackend.__name__}.add",
-    ) as mock_add:
+        "requests.Session.get", autospec=True, side_effect=DataContextError("not found")
+    ) as mock_get, mock.patch(
+        "requests.Session.post",
+        autospec=True,
+    ) as mock_post:
         context.add_or_update_expectation_suite(expectation_suite=suite)
 
-    mock_add.assert_called_once()
+    mock_get.assert_called_once()  # check if resource exists
+    mock_post.assert_called_once()  # persist resource
 
 
 @pytest.mark.unit
@@ -637,7 +641,7 @@ def test_expectation_suite_gx_cloud_identifier_requires_id_or_resource_name(
 @pytest.mark.unit
 @pytest.mark.cloud
 def test_add_or_update_expectation_suite_updates_existing_obj(
-    empty_base_data_context_in_cloud_mode: CloudDataContext,
+    empty_base_data_context_in_cloud_mode: CloudDataContext, mocked_get_by_name_response
 ):
     context = empty_base_data_context_in_cloud_mode
     mock_expectations_store_has_key.return_value = True
@@ -650,11 +654,14 @@ def test_add_or_update_expectation_suite_updates_existing_obj(
         f"{GXCloudStoreBackend.__module__}.{GXCloudStoreBackend.__name__}.has_key",
         return_value=True,
     ), mock.patch(
-        f"{GXCloudStoreBackend.__module__}.{GXCloudStoreBackend.__name__}.update",
-    ) as mock_update:
+        "requests.Session.get", autospec=True, side_effect=mocked_get_by_name_response
+    ) as mock_get, mock.patch(
+        "requests.Session.put", autospec=True
+    ) as mock_put:
         context.add_or_update_expectation_suite(expectation_suite=suite)
 
-    mock_update.assert_called_once()
+    mock_get.assert_called_once()  # check if resource exists
+    mock_put.assert_called_once()  # persist resource
 
 
 @pytest.mark.integration

tests/data_context/test_data_context_state_management.py

@@ -599,15 +599,11 @@ def test_add_expectation_suite_conflicting_args_failure(
 ):
     context = in_memory_data_context
 
-    with pytest.raises(ValueError) as e:
+    with pytest.raises(ValueError):
         context.add_expectation_suite(
             expectation_suite=suite, expectation_suite_name=suite_name
         )
 
-    assert (
-        "an existing expectation_suite or individual constructor arguments (but not both)"
-        in str(e.value)
-    )
     assert context.expectations_store.save_count == 0