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

Bump the minimum version of proto-plus to 1.22.3 #1774

Closed
parthea opened this issue Sep 18, 2023 · 0 comments · Fixed by #1863
Closed

Bump the minimum version of proto-plus to 1.22.3 #1774

parthea opened this issue Sep 18, 2023 · 0 comments · Fixed by #1863
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 Sep 18, 2023

I had to manually fix a google-cloud-python PR that had a comment looking like this:

proto-plus version 1.22.3 includes a fix to resolve an issue where marshal fails with cross api dependency.
https://github.com/googleapis/proto-plus-python/releases/tag/v1.22.3

With older versions of proto-plus, generated unit tests would fail with TypeError due to the issue mentioned above with marshal failing for cross api dependencies.

=================================== FAILURES ===================================
_________________________ test_list_crypto_keys_pager __________________________

transport_name = 'grpc'

    def test_list_crypto_keys_pager(transport_name: str = "grpc"):
        client = KeyDashboardServiceClient(
            credentials=ga_credentials.AnonymousCredentials,
            transport=transport_name,
        )
    
        # Mock the actual call within the gRPC stub, and fake the request.
        with mock.patch.object(type(client.transport.list_crypto_keys), "__call__") as call:
            # Set the response to a series of pages.
            call.side_effect = (
                key_dashboard_service.ListCryptoKeysResponse(
                    crypto_keys=[
                        resources.CryptoKey(),
                        resources.CryptoKey(),
                        resources.CryptoKey(),
                    ],
>                   next_page_token="abc",
                ),
                key_dashboard_service.ListCryptoKeysResponse(
                    crypto_keys=[],
                    next_page_token="def",
                ),
                key_dashboard_service.ListCryptoKeysResponse(
                    crypto_keys=[
                        resources.CryptoKey(),
                    ],
                    next_page_token="ghi",
                ),
                key_dashboard_service.ListCryptoKeysResponse(
                    crypto_keys=[
                        resources.CryptoKey(),
                        resources.CryptoKey(),
                    ],
                ),
                RuntimeError,
            )

tests/unit/gapic/kms_inventory_v1/test_key_dashboard_service.py:1001: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <[AttributeError('Unknown field for ListCryptoKeysResponse: _pb') raised in repr()] ListCryptoKeysResponse object at 0x7f144f26da10>
mapping = {'crypto_keys': [, , ], 'next_page_token': 'abc'}
ignore_unknown_fields = False
kwargs = {'crypto_keys': [, , ], 'next_page_token': 'abc'}
params = {'crypto_keys': [, , ], 'next_page_token': 'abc'}
marshal = <proto.marshal.marshal.Marshal object at 0x7f144f66b950>
key = 'next_page_token', value = 'abc', pb_value = 'abc'

    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/227
                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 google.cloud.kms.v1.CryptoKey got CryptoKey.

.nox/unit-3-7/lib/python3.7/site-packages/proto/message.py:599: 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 Sep 18, 2023
parthea added a commit to googleapis/google-cloud-python that referenced this issue Oct 26, 2023
@parthea parthea assigned ohmayr and unassigned dizcology Nov 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.

3 participants