Skip to content

Commit

Permalink
Add a capi conversion no-op method to thrift python structs/unions
Browse files Browse the repository at this point in the history
Summary: There's a generated python capi method on thrift-py3 structs to initialize from thrift python types. It's a very simple fix to add a no-op method to the base thrift python StructOrUnion class to get these unit tests to start passing now. Since the method is meant to initialize a thrift python struct with a thrift python struct, all that we need to do is return the passed in object (after a basic instance check to make sure no python shenanigans are going on).

Reviewed By: ahilger

Differential Revision: D68867840

fbshipit-source-id: 817381af3586cf7586532c6f94af676cfc16b241
  • Loading branch information
Filip Francetic authored and facebook-github-bot committed Jan 30, 2025
1 parent 81c9714 commit 0b16928
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
6 changes: 0 additions & 6 deletions thrift/lib/py3/test/auto_migrate/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ class Evil(Enum):
evil_cls.__module__, "thrift.lib.py3.test.auto_migrate.converter"
)

@brokenInAutoMigrate()
def test_simple_capi(self) -> None:
self.assert_simple(py3_types.Simple.from_python(self.make_simple_python()))

Expand Down Expand Up @@ -358,7 +357,6 @@ def test_nested(self) -> None:
nested.colorToSimpleMap[py3_types.Color.BLUE].color, py3_types.Color.BLUE
)

@brokenInAutoMigrate()
def test_nested_capi(self) -> None:
self.assertEqual(
self.make_nested_python()._to_py3(),
Expand All @@ -370,7 +368,6 @@ def test_simple_union(self) -> None:
self.assertEqual(simple_union.type, py3_types.Union.Type.intField)
self.assertEqual(simple_union.value, 42)

@brokenInAutoMigrate()
def test_simple_union_capi(self) -> None:
simple_union = py3_types.Union.from_python(python_types.Union(intField=42))
self.assertEqual(simple_union.type, py3_types.Union.Type.intField)
Expand All @@ -381,7 +378,6 @@ def test_union_with_py3_name_annotation(self) -> None:
self.assertEqual(simple_union.type, py3_types.Union.Type.name_)
self.assertEqual(simple_union.value, "myname")

@brokenInAutoMigrate()
def test_union_with_py3_name_annotation_capi(self) -> None:
simple_union = py3_types.Union.from_python(python_types.Union(name_="myname"))
self.assertEqual(simple_union.type, py3_types.Union.Type.name_)
Expand All @@ -392,7 +388,6 @@ def test_union_with_containers(self) -> None:
self.assertEqual(union_with_list.type, py3_types.Union.Type.intList)
self.assertEqual(union_with_list.value, [1, 2, 3])

@brokenInAutoMigrate()
def test_union_with_containers_capi(self) -> None:
union_with_list = py3_types.Union.from_python(
python_types.Union(intList=[1, 2, 3])
Expand All @@ -407,7 +402,6 @@ def test_complex_union(self) -> None:
self.assertEqual(complex_union.type, py3_types.Union.Type.simple_)
self.assertEqual(complex_union.simple_.intField, 42)

@brokenInAutoMigrate()
def test_complex_union_capi(self) -> None:
complex_union = py3_types.Union.from_python(
python_types.Union(
Expand Down
7 changes: 7 additions & 0 deletions thrift/lib/python/exceptions.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,13 @@ cdef class GeneratedError(Error):
)
self._fbthrift_populate_field_values()

@staticmethod
def from_python(obj: GeneratedError) -> GeneratedError:
if not isinstance(obj, GeneratedError):
raise TypeError(f'value {obj} expected to be a thrift-python Exception, was actually of type ' f'{type(obj)}')
return obj


@classmethod
def _fbthrift_auto_migrate_enabled(cls):
return False
Expand Down
6 changes: 6 additions & 0 deletions thrift/lib/python/types.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,12 @@ cdef class StructOrUnion:
cdef _fbthrift_get_field_value(self, int16_t index):
raise NotImplementedError("Not implemented on base StructOrUnion class")

@staticmethod
def from_python(obj: StructOrUnion) -> StructOrUnion:
if not isinstance(obj, StructOrUnion):
raise TypeError(f'value {obj} expected to be a thrift-python Struct or Union, was actually of type ' f'{type(obj)}')
return obj

@classmethod
def _fbthrift_auto_migrate_enabled(cls):
return False
Expand Down

0 comments on commit 0b16928

Please sign in to comment.