address comments

Author: hectorcast-db

databricks/sdk/common/types/fieldmask.py

@@ -1,15 +1,37 @@
 class FieldMask(object):
     """Class for FieldMask message type."""
+    # This is based on the base implementation from protobuf.
+    # https://pigweed.googlesource.com/third_party/github/protocolbuffers/protobuf/+/HEAD/python/google/protobuf/internal/field_mask.py
+    # The original implementation only works with proto generated classes.
+    # Since our classes are not generated from proto files, we need to implement it manually.
 
-    def ToJsonString(self):
+    def __init__(self):
+        """Initialize FieldMask with empty paths."""
+        self.paths = []
+
+    def ToJsonString(self) -> str:
         """Converts FieldMask to string."""
         return ",".join(self.paths)
 
-    def FromJsonString(self, value):
+    def FromJsonString(self, value: str) -> None:
         """Converts string to FieldMask."""
         if not isinstance(value, str):
             raise ValueError("FieldMask JSON value not a string: {!r}".format(value))
         if value:
             self.paths = value.split(",")
         else:
             self.paths = []
+
+    def __eq__(self, other) -> bool:
+        """Check equality based on paths."""
+        if not isinstance(other, FieldMask):
+            return False
+        return self.paths == other.paths
+
+    def __hash__(self) -> int:
+        """Hash based on paths tuple."""
+        return hash(tuple(self.paths))
+
+    def __repr__(self) -> str:
+        """String representation for debugging."""
+        return f"FieldMask(paths={self.paths})"

databricks/sdk/service/_internal.py

@@ -52,6 +52,11 @@ def _escape_multi_segment_path_parameter(param: str) -> str:
 
 
 def _timestamp(d: Dict[str, any], field: str) -> Optional[Timestamp]:
+    """
+    Helper function to convert a timestamp string to a Timestamp object.
+    It takes a dictionary and a field name, and returns a Timestamp object.
+    The field name is the key in the dictionary that contains the timestamp string.
+    """
     if field not in d or not d[field]:
         return None
     ts = Timestamp()
@@ -60,6 +65,11 @@ def _timestamp(d: Dict[str, any], field: str) -> Optional[Timestamp]:
 
 
 def _repeated_timestamp(d: Dict[str, any], field: str) -> Optional[List[Timestamp]]:
+    """
+    Helper function to convert a list of timestamp strings to a list of Timestamp objects.
+    It takes a dictionary and a field name, and returns a list of Timestamp objects.
+    The field name is the key in the dictionary that contains the list of timestamp strings.
+    """
     if field not in d or not d[field]:
         return None
     result = []
@@ -71,6 +81,11 @@ def _repeated_timestamp(d: Dict[str, any], field: str) -> Optional[List[Timestam
 
 
 def _duration(d: Dict[str, any], field: str) -> Optional[Duration]:
+    """
+    Helper function to convert a duration string to a Duration object.
+    It takes a dictionary and a field name, and returns a Duration object.
+    The field name is the key in the dictionary that contains the duration string.
+    """
     if field not in d or not d[field]:
         return None
     dur = Duration()
@@ -79,6 +94,11 @@ def _duration(d: Dict[str, any], field: str) -> Optional[Duration]:
 
 
 def _repeated_duration(d: Dict[str, any], field: str) -> Optional[List[Duration]]:
+    """
+    Helper function to convert a list of duration strings to a list of Duration objects.
+    It takes a dictionary and a field name, and returns a list of Duration objects.
+    The field name is the key in the dictionary that contains the list of duration strings.
+    """
     if field not in d or not d[field]:
         return None
     result = []
@@ -90,6 +110,11 @@ def _repeated_duration(d: Dict[str, any], field: str) -> Optional[List[Duration]
 
 
 def _fieldmask(d: Dict[str, any], field: str) -> Optional[FieldMask]:
+    """
+    Helper function to convert a fieldmask string to a FieldMask object.
+    It takes a dictionary and a field name, and returns a FieldMask object.
+    The field name is the key in the dictionary that contains the fieldmask string.
+    """
     if field not in d or not d[field]:
         return None
     fm = FieldMask()
@@ -98,6 +123,11 @@ def _fieldmask(d: Dict[str, any], field: str) -> Optional[FieldMask]:
 
 
 def _repeated_fieldmask(d: Dict[str, any], field: str) -> Optional[List[FieldMask]]:
+    """
+    Helper function to convert a list of fieldmask strings to a list of FieldMask objects.
+    It takes a dictionary and a field name, and returns a list of FieldMask objects.
+    The field name is the key in the dictionary that contains the list of fieldmask strings.
+    """
     if field not in d or not d[field]:
         return None
     result = []

pyproject.toml

@@ -27,6 +27,7 @@ classifiers = [
 dependencies = [
     "requests>=2.28.1,<3",
     "google-auth~=2.0",
+    "protobuf>=4.21.0,<7.0",
 ]
 
 [project.urls]

tests/test_internal.py

@@ -77,35 +77,38 @@ def test_timestamp(input_dict, field_name, expected_result, expected_json, descr
         assert result is None
     else:
         assert isinstance(result, Timestamp)
-        assert result.ToJsonString() == expected_json
+        ts = Timestamp()
+        ts.FromJsonString(expected_json)
+        assert result == ts
 
 
 @pytest.mark.parametrize(
-    "input_dict,field_name,expected_length,expected_json_list,description",
+    "input_dict,field_name,expected_json_list,description",
     [
         (
             {"field": ["2023-01-01T12:00:00Z", "2023-01-02T12:00:00Z"]},
             "field",
-            2,
             ["2023-01-01T12:00:00Z", "2023-01-02T12:00:00Z"],
             "valid repeated timestamps",
         ),
-        ({}, "field", None, None, "missing field"),
-        ({"field": None}, "field", None, None, "None value"),
-        ({"field": []}, "field", None, None, "empty list"),
+        ({}, "field", [], "missing field"),
+        ({"field": None}, "field", None, "None value"),
+        ({"field": []}, "field", [], "empty list"),
     ],
 )
-def test_repeated_timestamp(input_dict, field_name, expected_length, expected_json_list, description):
+def test_repeated_timestamp(input_dict, field_name, expected_json_list, description):
     """Test _repeated_timestamp function with various input scenarios."""
     result = _repeated_timestamp(input_dict, field_name)
 
-    if expected_length is None:
+    if expected_json_list is None or len(expected_json_list) == 0:
         assert result is None
     else:
-        assert len(result) == expected_length
+        assert len(result) == len(expected_json_list)
         assert all(isinstance(ts, Timestamp) for ts in result)
         for i, expected_json in enumerate(expected_json_list):
-            assert result[i].ToJsonString() == expected_json
+            ts = Timestamp()
+            ts.FromJsonString(expected_json)
+            assert result[i] == ts
 
 
 @pytest.mark.parametrize(
@@ -125,29 +128,33 @@ def test_duration(input_dict, field_name, expected_result, expected_json, descri
         assert result is None
     else:
         assert isinstance(result, Duration)
-        assert result.ToJsonString() == expected_json
+        dur = Duration()
+        dur.FromJsonString(expected_json)
+        assert result == dur
 
 
 @pytest.mark.parametrize(
-    "input_dict,field_name,expected_length,expected_json_list,description",
+    "input_dict,field_name,expected_json_list,description",
     [
-        ({"field": ["3600s", "7200s"]}, "field", 2, ["3600s", "7200s"], "valid repeated durations"),
-        ({}, "field", None, None, "missing field"),
-        ({"field": None}, "field", None, None, "None value"),
-        ({"field": []}, "field", None, None, "empty list"),
+        ({"field": ["3600s", "7200s"]}, "field", ["3600s", "7200s"], "valid repeated durations"),
+        ({}, "field", [], "missing field"),
+        ({"field": None}, "field", None, "None value"),
+        ({"field": []}, "field", [], "empty list"),
     ],
 )
-def test_repeated_duration(input_dict, field_name, expected_length, expected_json_list, description):
+def test_repeated_duration(input_dict, field_name, expected_json_list, description):
     """Test _repeated_duration function with various input scenarios."""
     result = _repeated_duration(input_dict, field_name)
 
-    if expected_length is None:
+    if expected_json_list is None or len(expected_json_list) == 0:
         assert result is None
     else:
-        assert len(result) == expected_length
+        assert len(result) == len(expected_json_list)
         assert all(isinstance(dur, Duration) for dur in result)
         for i, expected_json in enumerate(expected_json_list):
-            assert result[i].ToJsonString() == expected_json
+            dur = Duration()
+            dur.FromJsonString(expected_json)
+            assert result[i] == dur
 
 
 @pytest.mark.parametrize(
@@ -167,32 +174,35 @@ def test_fieldmask(input_dict, field_name, expected_result, expected_json, descr
         assert result is None
     else:
         assert isinstance(result, FieldMask)
-        assert result.ToJsonString() == expected_json
+        fm = FieldMask()
+        fm.FromJsonString(expected_json)
+        assert result == fm
 
 
 @pytest.mark.parametrize(
-    "input_dict,field_name,expected_length,expected_json_list,description",
+    "input_dict,field_name,expected_json_list,description",
     [
         (
             {"field": ["path1,path2", "path3,path4"]},
             "field",
-            2,
             ["path1,path2", "path3,path4"],
             "valid repeated fieldmasks",
         ),
-        ({}, "field", None, None, "missing field"),
-        ({"field": None}, "field", None, None, "None value"),
-        ({"field": []}, "field", None, None, "empty list"),
+        ({}, "field", [], "missing field"),
+        ({"field": None}, "field", None, "None value"),
+        ({"field": []}, "field", [], "empty list"),
     ],
 )
-def test_repeated_fieldmask(input_dict, field_name, expected_length, expected_json_list, description):
+def test_repeated_fieldmask(input_dict, field_name, expected_json_list, description):
     """Test _repeated_fieldmask function with various input scenarios."""
     result = _repeated_fieldmask(input_dict, field_name)
 
-    if expected_length is None:
+    if expected_json_list is None or len(expected_json_list) == 0:
         assert result is None
     else:
-        assert len(result) == expected_length
+        assert len(result) == len(expected_json_list)
         assert all(isinstance(fm, FieldMask) for fm in result)
         for i, expected_json in enumerate(expected_json_list):
-            assert result[i].ToJsonString() == expected_json
+            fm = FieldMask()
+            fm.FromJsonString(expected_json)
+            assert result[i] == fm