Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Marshal fails with TypeError when using cross api message #349

Closed
parthea opened this issue Mar 15, 2023 · 0 comments · Fixed by #348
Closed

Marshal fails with TypeError when using cross api message #349

parthea opened this issue Mar 15, 2023 · 0 comments · Fixed by #348
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@parthea
Copy link
Contributor

parthea commented Mar 15, 2023

See test code here e123ad8, with corresponding failure here.

See the full stack trace below

________________________ test_module_package_cross_api _________________________

    def test_module_package_cross_api():
        sys.modules[__name__].__protobuf__ = proto.module(package="spam.eggs.v1")
        try:
    
            class Baz(proto.Message):
                foo = proto.RepeatedField(proto.INT64, number=1)
    
    
            marshal = proto.Marshal(name="spam.eggs.v1")
    
            assert Baz.meta.package == "spam.eggs.v1"
            assert Baz.pb() in marshal._rules
    
            sys.modules[__name__].__protobuf__ = proto.module(package="ham.pancakes.v1")
    
            class AnotherMessage(proto.Message):
                qux = proto.Field(proto.MESSAGE, number=1, message=Baz)
    
            marshal = proto.Marshal(name="ham.pancakes.v1")
    
            assert AnotherMessage.meta.package == "ham.pancakes.v1"
            assert AnotherMessage.pb() in marshal._rules
            # Confirm that Baz.pb() is no longer present in marshal._rules
            assert Baz.pb() not in marshal._rules
    
            # Test using multiple packages together
>           msg = AnotherMessage(qux=Baz())

tests/test_modules.py:65: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <[AttributeError('Unknown field for AnotherMessage: _pb') raised in repr()] AnotherMessage object at 0x7f112bbc07d0>
mapping = {'qux': }, ignore_unknown_fields = False, kwargs = {'qux': }
params = {'qux': }
marshal = <proto.marshal.marshal.Marshal object at 0x7f112bbc0910>, key = 'qux'
value = , pb_value = 

    def __init__(
        self,
        mapping=None,
        *,
        ignore_unknown_fields=False,
        **kwargs,
    ):
        # We accept several things for `mapping`:
        #   * An instance of this class.
        #   * An instance of the underlying protobuf descriptor class.
        #   * A dict
        #   * Nothing (keyword arguments only).
        if mapping is None:
            if not kwargs:
                # Special fast path for empty construction.
                super().__setattr__("_pb", self._meta.pb())
                return
    
            mapping = kwargs
        elif isinstance(mapping, self._meta.pb):
            # Make a copy of the mapping.
            # This is a constructor for a new object, so users will assume
            # that it will not have side effects on the arguments being
            # passed in.
            #
            # The `wrap` method on the metaclass is the public API for taking
            # ownership of the passed in protobuf object.
            mapping = copy.deepcopy(mapping)
            if kwargs:
                mapping.MergeFrom(self._meta.pb(**kwargs))
    
            super().__setattr__("_pb", mapping)
            return
        elif isinstance(mapping, type(self)):
            # Just use the above logic on mapping's underlying pb.
            self.__init__(mapping=mapping._pb, **kwargs)
            return
        elif isinstance(mapping, collections.abc.Mapping):
            # Can't have side effects on mapping.
            mapping = copy.copy(mapping)
            # kwargs entries take priority for duplicate keys.
            mapping.update(kwargs)
        else:
            # Sanity check: Did we get something not a map? Error if so.
            raise TypeError(
                "Invalid constructor input for %s: %r"
                % (
                    self.__class__.__name__,
                    mapping,
                )
            )
    
        params = {}
        # Update the mapping to address any values that need to be
        # coerced.
        marshal = self._meta.marshal
        for key, value in mapping.items():
            (key, pb_type) = self._get_pb_type_from_key(key)
            if pb_type is None:
                if ignore_unknown_fields:
                    continue
    
                raise ValueError(
                    "Unknown field for {}: {}".format(self.__class__.__name__, key)
                )
    
            try:
                pb_value = marshal.to_proto(pb_type, value)
            except ValueError:
                # Underscores may be appended to field names
                # that collide with python or proto-plus keywords.
                # In case a key only exists with a `_` suffix, coerce the key
                # to include the `_` suffix. It's not possible to
                # natively define the same field with a trailing underscore in protobuf.
                # See related issue
                # https://github.com/googleapis/python-api-core/issues/2[27](https://github.com/googleapis/proto-plus-python/actions/runs/4426034682/jobs/7761831001#step:6:28)
                if isinstance(value, dict):
                    if _upb:
                        # In UPB, pb_type is MessageMeta which doesn't expose attrs like it used to in Python/CPP.
                        keys_to_update = [
                            item
                            for item in value
                            if item not in pb_type.DESCRIPTOR.fields_by_name
                            and f"{item}_" in pb_type.DESCRIPTOR.fields_by_name
                        ]
                    else:
                        keys_to_update = [
                            item
                            for item in value
                            if not hasattr(pb_type, item)
                            and hasattr(pb_type, f"{item}_")
                        ]
                    for item in keys_to_update:
                        value[f"{item}_"] = value.pop(item)
    
                pb_value = marshal.to_proto(pb_type, value)
    
            if pb_value is not None:
                params[key] = pb_value
    
        # Create the internal protocol buffer.
>       super().__setattr__("_pb", self._meta.pb(**params))
E       TypeError: Parameter to MergeFrom() must be instance of same class: expected spam.eggs.v1.Baz got Baz.

proto/message.py:[60](https://github.com/googleapis/proto-plus-python/actions/runs/4426034682/jobs/7761831001#step:6:61)4: TypeError
@parthea parthea added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Mar 15, 2023
@parthea parthea self-assigned this Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant