Skip to content

Commit

Permalink
Fix name conflict between abc.ABC.register and fields named register.
Browse files Browse the repository at this point in the history 
Summary:
# What?
If a thrift type has a field named `register`, the call to register it with the abstract base class fails because the python class has a property named register.

`TestRegisterAsField` in `special_cases.thrift` is a test for this case.

```
buck2 test fbcode//thrift/lib/python/test:special_cases_test
```
will fail due to the error
```
TypeError: 'property' object is not callable
```

# What to do?
Per python's official documentation, `abc.ABCMeta` provides [register](https://docs.python.org/3/library/abc.html#abc.ABCMeta.register). Rather than the `register` method available on the abstract base class, use this method directly to register the virtual subclass.

# Generate fixtures
```
buck2 run fbcode//thrift/compiler/test:build_fixtures
```

Reviewed By: Filip-F

Differential Revision: D66739239

fbshipit-source-id: 72fb5948b7b633559458a8589fc28b1f6512a10e
  • Loading branch information
@facebook-github-bot
Satish Kumar authored and facebook-github-bot committed Dec 5, 2024
1 parent 3472a20 commit b906a1f
Show file tree
Hide file tree
Showing 35 changed files with 450 additions and 204 deletions.
29 changes: 17 additions & 12 deletions thrift/compiler/generate/templates/python/types/types.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import folly.iobuf as _fbthrift_iobuf
{{> types/set_current_module}}

{{#program:enable_abstract_types?}}
from abc import ABCMeta as _fbthrift_ABCMeta
import {{program:module_path}}.thrift_abstract_types as _fbthrift_abstract_types
{{/program:enable_abstract_types?}}
import {{program:base_library_package}}.types as _fbthrift_python_types
Expand All @@ -48,18 +49,7 @@ import {{module_path}}
{{#program:structs}}


{{#program:enable_abstract_types?}}{{!
TODO(T208096866)
If the class uses the abstract class as a direct baseclass, it results in the error below.
TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict)
subclass of the metaclasses of all its bases
Until we resolve the above error, register the class as virtual subclass of the abstract class.
}}{{^struct:exception?}}
@_fbthrift_abstract_types.{{> structs/unadapted_name}}.register
{{/struct:exception?}}{{!
}}{{/program:enable_abstract_types?}}{{!
}}class {{> structs/unadapted_name}}{{!
class {{> structs/unadapted_name}}{{!
}}(metaclass={{!
}}{{#struct:union?}}{{> types/union_metaclass}}{{/struct:union?}}{{!
}}{{^struct:union?}}{{!
Expand Down Expand Up @@ -181,6 +171,21 @@ Until we resolve the above error, register the class as virtual subclass of the
py_asyncio_types = importlib.import_module("{{program:py_asyncio_module_path}}.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.{{> structs/unadapted_name}}, self)
{{/struct:legacy_api?}}
{{#program:enable_abstract_types?}}{{!
TODO(T208096866)
If the class uses the abstract class as a direct baseclass, it results in the error below.
TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict)
subclass of the metaclasses of all its bases
Until we resolve the above error, register the class as virtual subclass of the abstract class.
To workaround the problem that a thrift field named register will cause register to become a property
on the abstract type and prevent the ability to use it to register a virtual subclass,
use abc.ABCMeta.register(cls, subclass) to register.
}}{{^struct:exception?}}

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.{{> structs/unadapted_name}}, {{> structs/unadapted_name}})
{{/struct:exception?}}
{{/program:enable_abstract_types?}}
{{#struct:has_adapter?}}


Expand Down
19 changes: 13 additions & 6 deletions ...tal_generate_abstract_types/gen-python/test/fixtures/basic/module/thrift_mutable_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import folly.iobuf as _fbthrift_iobuf

import test.fixtures.basic.module.thrift_mutable_types as _fbthrift_current_module
from abc import ABCMeta as _fbthrift_ABCMeta
import test.fixtures.basic.module.thrift_abstract_types as _fbthrift_abstract_types
import thrift.python.types as _fbthrift_python_types
import thrift.python.mutable_types as _fbthrift_python_mutable_types
Expand All @@ -22,7 +23,6 @@



@_fbthrift_abstract_types.MyStruct.register
class MyStruct(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -164,8 +164,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyStruct, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyStruct, MyStruct)


@_fbthrift_abstract_types.Containers.register
class Containers(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -241,8 +242,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.Containers, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.Containers, Containers)


@_fbthrift_abstract_types.MyDataItem.register
class MyDataItem(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
)
Expand Down Expand Up @@ -285,8 +287,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyDataItem, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyDataItem, MyDataItem)


@_fbthrift_abstract_types.MyUnion.register
class MyUnion(metaclass=_fbthrift_python_mutable_types.MutableUnionMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -376,6 +379,8 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyUnion, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyUnion, MyUnion)


class MyException(metaclass=_fbthrift_python_mutable_exceptions.MutableGeneratedErrorMeta):
_fbthrift_SPEC = (
Expand Down Expand Up @@ -564,7 +569,6 @@ def _to_py_deprecated(self):
return thrift.util.converter.to_py_struct(py_asyncio_types.MyExceptionWithMessage, self)


@_fbthrift_abstract_types.ReservedKeyword.register
class ReservedKeyword(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -618,8 +622,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.ReservedKeyword, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.ReservedKeyword, ReservedKeyword)


@_fbthrift_abstract_types.UnionToBeRenamed.register
class UnionToBeRenamed(metaclass=_fbthrift_python_mutable_types.MutableUnionMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -676,6 +681,8 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.UnionToBeRenamed, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.UnionToBeRenamed, UnionToBeRenamed)

from test.fixtures.basic.module.thrift_enums import *

_fbthrift_all_enums = [
Expand Down
19 changes: 13 additions & 6 deletions ...xperimental_generate_abstract_types/gen-python/test/fixtures/basic/module/thrift_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
import folly.iobuf as _fbthrift_iobuf

import test.fixtures.basic.module.thrift_types as _fbthrift_current_module
from abc import ABCMeta as _fbthrift_ABCMeta
import test.fixtures.basic.module.thrift_abstract_types as _fbthrift_abstract_types
import thrift.python.types as _fbthrift_python_types
import thrift.python.exceptions as _fbthrift_python_exceptions



@_fbthrift_abstract_types.MyStruct.register
class MyStruct(metaclass=_fbthrift_python_types.StructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -157,8 +157,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyStruct, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyStruct, MyStruct)


@_fbthrift_abstract_types.Containers.register
class Containers(metaclass=_fbthrift_python_types.StructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -233,8 +234,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.Containers, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.Containers, Containers)


@_fbthrift_abstract_types.MyDataItem.register
class MyDataItem(metaclass=_fbthrift_python_types.StructMeta):
_fbthrift_SPEC = (
)
Expand Down Expand Up @@ -276,8 +278,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyDataItem, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyDataItem, MyDataItem)


@_fbthrift_abstract_types.MyUnion.register
class MyUnion(metaclass=_fbthrift_python_types.UnionMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -366,6 +369,8 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyUnion, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyUnion, MyUnion)


class MyException(metaclass=_fbthrift_python_exceptions.GeneratedErrorMeta):
_fbthrift_SPEC = (
Expand Down Expand Up @@ -552,7 +557,6 @@ def _to_py_deprecated(self):
return thrift.util.converter.to_py_struct(py_asyncio_types.MyExceptionWithMessage, self)


@_fbthrift_abstract_types.ReservedKeyword.register
class ReservedKeyword(metaclass=_fbthrift_python_types.StructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -605,8 +609,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.ReservedKeyword, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.ReservedKeyword, ReservedKeyword)


@_fbthrift_abstract_types.UnionToBeRenamed.register
class UnionToBeRenamed(metaclass=_fbthrift_python_types.UnionMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -662,6 +667,8 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.UnionToBeRenamed, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.UnionToBeRenamed, UnionToBeRenamed)

# This unfortunately has to be down here to prevent circular imports
import test.fixtures.basic.module.thrift_metadata
from test.fixtures.basic.module.thrift_enums import *
Expand Down
34 changes: 23 additions & 11 deletions ...out/python_experimental_generate_abstract_types/gen-python/module/thrift_mutable_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import folly.iobuf as _fbthrift_iobuf

import module.thrift_mutable_types as _fbthrift_current_module
from abc import ABCMeta as _fbthrift_ABCMeta
import module.thrift_abstract_types as _fbthrift_abstract_types
import thrift.python.types as _fbthrift_python_types
import thrift.python.mutable_types as _fbthrift_python_mutable_types
Expand All @@ -22,7 +23,6 @@



@_fbthrift_abstract_types.MyStructFloatFieldThrowExp.register
class MyStructFloatFieldThrowExp(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -109,8 +109,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyStructFloatFieldThrowExp, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyStructFloatFieldThrowExp, MyStructFloatFieldThrowExp)


@_fbthrift_abstract_types.MyStructMapFloatThrowExp.register
class MyStructMapFloatThrowExp(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -175,8 +176,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyStructMapFloatThrowExp, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyStructMapFloatThrowExp, MyStructMapFloatThrowExp)


@_fbthrift_abstract_types.MyStruct.register
class MyStruct(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -527,8 +529,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyStruct, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyStruct, MyStruct)


@_fbthrift_abstract_types.SimpleStruct.register
class SimpleStruct(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -593,8 +596,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.SimpleStruct, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.SimpleStruct, SimpleStruct)


@_fbthrift_abstract_types.defaultStruct.register
class defaultStruct(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -879,8 +883,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.defaultStruct, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.defaultStruct, defaultStruct)


@_fbthrift_abstract_types.MyStructTypeDef.register
class MyStructTypeDef(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -1022,8 +1027,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyStructTypeDef, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyStructTypeDef, MyStructTypeDef)


@_fbthrift_abstract_types.MyDataItem.register
class MyDataItem(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
)
Expand Down Expand Up @@ -1066,8 +1072,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyDataItem, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyDataItem, MyDataItem)


@_fbthrift_abstract_types.MyUnion.register
class MyUnion(metaclass=_fbthrift_python_mutable_types.MutableUnionMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -1179,8 +1186,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyUnion, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyUnion, MyUnion)


@_fbthrift_abstract_types.MyUnionFloatFieldThrowExp.register
class MyUnionFloatFieldThrowExp(metaclass=_fbthrift_python_mutable_types.MutableUnionMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -1270,8 +1278,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.MyUnionFloatFieldThrowExp, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.MyUnionFloatFieldThrowExp, MyUnionFloatFieldThrowExp)


@_fbthrift_abstract_types.ComplexNestedStruct.register
class ComplexNestedStruct(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -1512,8 +1521,9 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.ComplexNestedStruct, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.ComplexNestedStruct, ComplexNestedStruct)


@_fbthrift_abstract_types.TypeRemapped.register
class TypeRemapped(metaclass=_fbthrift_python_mutable_types.MutableStructMeta):
_fbthrift_SPEC = (
_fbthrift_python_types.FieldInfo(
Expand Down Expand Up @@ -1600,6 +1610,8 @@ def _to_py_deprecated(self):
py_asyncio_types = importlib.import_module("module.ttypes")
return thrift.util.converter.to_py_struct(py_asyncio_types.TypeRemapped, self)

_fbthrift_ABCMeta.register(_fbthrift_abstract_types.TypeRemapped, TypeRemapped)


class emptyXcep(metaclass=_fbthrift_python_mutable_exceptions.MutableGeneratedErrorMeta):
_fbthrift_SPEC = (
Expand Down
Loading

0 comments on commit b906a1f

Please sign in to comment.