Skip to content

Commit 6d41abd

Browse files
committed
Added additional checking of duplicate iceberg properties and testing
1 parent 320e14f commit 6d41abd

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group_manager.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def validate_property_keys(self):
6161
# Check for no duplicate keys
6262
if len(set(self.properties.keys())) != len(self.properties.keys()):
6363
raise ValueError(
64-
f"Invalide duplicate properties. Please only have 1 of each property."
64+
f"Invalid duplicate properties. Please only have 1 of each property."
6565
)
6666
return self
6767

@@ -819,6 +819,12 @@ def _update_iceberg_properties(
819819
f"Approved properties are: {_APPROVED_ICEBERG_PROPERTIES}"
820820
)
821821

822+
# Check for no duplicate keys
823+
if len(set(iceberg_properties.properties.keys())) != len(iceberg_properties.properties.keys()):
824+
raise ValueError(
825+
f"Invalid duplicate properties. Please only have 1 of each property."
826+
)
827+
822828
result = self._get_iceberg_properties(session=session, region=region)
823829
database_name = result["database_name"]
824830
table_name = result["table_name"]

sagemaker-mlops/tests/unit/sagemaker/mlops/feature_store/test_iceberg_properties.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ def test_error_message_contains_invalid_key_names(self):
5656
with pytest.raises(ValueError, match="fake.property"):
5757
IcebergProperties(properties={"fake.property": "value"})
5858

59+
def test_duplicate_keys_raises_error(self):
60+
"""Test that duplicate property keys raise ValueError."""
61+
config = IcebergProperties(properties={"write.target-file-size-bytes": "536870912"})
62+
mock_props = MagicMock()
63+
mock_props.keys.return_value = [
64+
"write.target-file-size-bytes",
65+
"write.target-file-size-bytes",
66+
]
67+
object.__setattr__(config, "properties", mock_props)
68+
with pytest.raises(ValueError, match="Invalid duplicate properties"):
69+
config.validate_property_keys()
70+
71+
def test_no_duplicate_keys_passes(self):
72+
"""Test that unique approved keys pass duplicate validation."""
73+
config = IcebergProperties(properties={"write.target-file-size-bytes": "536870912"})
74+
result = config.validate_property_keys()
75+
assert result is config
76+
5977

6078
class TestGetIcebergProperties:
6179
"""Tests for get_iceberg_properties method."""
@@ -278,6 +296,20 @@ def test_raises_runtime_error_on_update_table_client_error(self):
278296
with pytest.raises(RuntimeError, match="Failed to update Iceberg properties"):
279297
self.fg._update_iceberg_properties(iceberg_properties=props)
280298

299+
def test_raises_error_on_duplicate_keys(self):
300+
"""Test ValueError when iceberg_properties has duplicate keys."""
301+
props = IcebergProperties(properties={"write.target-file-size-bytes": "536870912"})
302+
mock_props = MagicMock()
303+
mock_props.keys.return_value = [
304+
"write.target-file-size-bytes",
305+
"write.target-file-size-bytes",
306+
]
307+
mock_props.__bool__ = lambda self: True
308+
object.__setattr__(props, "properties", mock_props)
309+
310+
with pytest.raises(ValueError, match="Invalid duplicate properties"):
311+
self.fg._update_iceberg_properties(iceberg_properties=props)
312+
281313

282314
class TestCreateWithIcebergProperties:
283315
"""Tests for create() method with iceberg_properties parameter."""

0 commit comments

Comments
 (0)