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

Method signature elements that are repeated fields are not handled properly #413

Closed
software-dov opened this issue May 11, 2020 · 2 comments · Fixed by #445 or #1056
Closed

Method signature elements that are repeated fields are not handled properly #413

software-dov opened this issue May 11, 2020 · 2 comments · Fixed by #445 or #1056
Assignees
Labels
generator Bugs, features, and so forth pertaining to the generated client surface priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@software-dov
Copy link
Contributor

Minimal reproducible code:

service MyService {
  rpc MyMethod(MethodRequest) returns (MethodResponse) {
  	option (google.api.method_signature) = "inputs,parameters";
  }
}

message MethodRequest {
  repeated google.protobuf.Value inputs = 1;
}

message MethodResponse {
  google.protobuf.Value output = 1;
}

In the portion of the client method that constructs the request, the following lines occur

 if inputs is not None:
            request.inputs = inputs

This causes protobuf to raise an error because request.intputs is a repeated field.
The solution is to do the following instead:

  if inputs is not None:
          request.inputs.extend(inputs)
@software-dov software-dov 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. generator Bugs, features, and so forth pertaining to the generated client surface labels May 11, 2020
@miraleung miraleung self-assigned this Jun 5, 2020
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Sep 3, 2020
@software-dov
Copy link
Contributor Author

Pretty sure this has been fixed in the templates, but can reopen if it's not actually the case.

@parthea parthea reopened this Jul 1, 2021
@parthea
Copy link
Contributor

parthea commented Jul 1, 2021

This is still an issue in googleapis/python-aiplatform. The owlbot.py file has a workaround. I removed the workaround and the sample tests failed with TypeError. See the build log here. For example :

_________________ test_ucaip_generated_explain_tabular_sample __________________

capsys = <_pytest.capture.CaptureFixture object at 0x7f31d88a1a10>

    def test_ucaip_generated_explain_tabular_sample(capsys):

        explain_tabular_sample.explain_tabular_sample(
>           instance_dict=INSTANCE, project=PROJECT_ID, endpoint_id=ENDPOINT_ID
        )

explain_tabular_sample_test.py:33:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
explain_tabular_sample.py:45: in explain_tabular_sample
    endpoint=endpoint, instances=instances, parameters=parameters
../../google/cloud/aiplatform_v1beta1/services/prediction_service/client.py:562: in explain
    request.instances = instances
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = endpoint: "projects/ucaip-sample-tests/locations/us-central1/endpoints/4966625964059525120"

key = 'instances'
value = [struct_value {
  fields {
    key: "petal_length"
    value {
      string_value: "1.4"
    }
  }
  fields {
    key:...    string_value: "5.1"
    }
  }
  fields {
    key: "sepal_width"
    value {
      string_value: "2.8"
    }
  }
}
]

    def __setattr__(self, key, value):
        """Set the value on the given field.

        For well-known protocol buffer types which are marshalled, either
        the protocol buffer object or the Python equivalent is accepted.
        """
        if key[0] == "_":
            return super().__setattr__(key, value)
        marshal = self._meta.marshal
        pb_type = self._meta.fields[key].pb_type
        pb_value = marshal.to_proto(pb_type, value)

        # Clear the existing field.
        # This is the only way to successfully write nested falsy values,
        # because otherwise MergeFrom will no-op on them.
        self._pb.ClearField(key)

        # Merge in the value being set.
        if pb_value is not None:
>           self._pb.MergeFrom(self._meta.pb(**{key: pb_value}))
E           TypeError: Value must be iterable

.nox/py-3-7/lib/python3.7/site-packages/proto/message.py:641: TypeError

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jul 4, 2021
@vchudnov-g vchudnov-g assigned software-dov and unassigned miraleung Oct 28, 2021
@software-dov software-dov linked a pull request Oct 29, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Bugs, features, and so forth pertaining to the generated client surface priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
4 participants