From 218d8196ce96a2ee02062066aa81730329a643b4 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Mon, 12 Oct 2026 12:05:48 -0500 Subject: [PATCH] fix(mlops): raise when the FeatureGroupManager.update() API call fails FeatureGroupManager.update() caught every exception from the parent FeatureGroup.update() (UpdateFeatureGroup), logged it and carried on: the Iceberg property update still ran and the method returned self as if the update had succeeded. ValidationException, ResourceInUse, AccessDenied and similar errors were therefore only visible in the logs. The Iceberg branch right below already re-raises after logging. Re-raise after logging, so the caller sees the failure and the Iceberg properties are not updated on a failed update. --- .../feature_store/feature_group_manager.py | 1 + .../feature_store/test_iceberg_properties.py | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group_manager.py b/sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group_manager.py index c4e22a03d8..b5f4d19843 100644 --- a/sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group_manager.py +++ b/sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group_manager.py @@ -1136,6 +1136,7 @@ def update( logger.error( f"Feature group '{self.feature_group_name}' was not updated successfully: {e}" ) + raise # Update Iceberg properties if requested if iceberg_properties is not None and iceberg_properties.properties: diff --git a/sagemaker-mlops/tests/unit/sagemaker/mlops/feature_store/test_iceberg_properties.py b/sagemaker-mlops/tests/unit/sagemaker/mlops/feature_store/test_iceberg_properties.py index 3defc0a9f6..04d7df25fc 100644 --- a/sagemaker-mlops/tests/unit/sagemaker/mlops/feature_store/test_iceberg_properties.py +++ b/sagemaker-mlops/tests/unit/sagemaker/mlops/feature_store/test_iceberg_properties.py @@ -792,6 +792,35 @@ def test_skips_parent_update_when_only_iceberg_properties( mock_update_iceberg.assert_called_once() assert result == fg + @patch.object(FeatureGroupManager, "_update_iceberg_properties") + @patch("sagemaker.core.resources.Base.get_sagemaker_client") + def test_parent_update_failure_is_raised(self, mock_get_client, mock_update_iceberg): + """A failed UpdateFeatureGroup must surface, and Iceberg properties must not be updated.""" + from botocore.exceptions import ClientError + from sagemaker.core.shapes import OfflineStoreConfig, S3StorageConfig, DataCatalogConfig + + mock_client = MagicMock() + mock_client.update_feature_group.side_effect = ClientError( + {"Error": {"Code": "ValidationException", "Message": "bad throughput"}}, + "UpdateFeatureGroup", + ) + mock_get_client.return_value = mock_client + + fg = FeatureGroupManager(feature_group_name="test-fg") + fg.offline_store_config = OfflineStoreConfig( + s3_storage_config=S3StorageConfig(s3_uri="s3://bucket/path"), + data_catalog_config=DataCatalogConfig( + catalog="AwsDataCatalog", database="db", table_name="tbl" + ), + table_format="Iceberg", + ) + iceberg_props = IcebergProperties(properties={"write.target-file-size-bytes": "536870912"}) + + with pytest.raises(ClientError, match="bad throughput"): + fg.update(feature_additions=[], iceberg_properties=iceberg_props) + + mock_update_iceberg.assert_not_called() + @patch.object(FeatureGroupManager, "_update_iceberg_properties") @patch.object(FeatureGroupManager, "refresh") @patch("sagemaker.core.resources.Base.get_sagemaker_client")