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

Nested message produces different proto schema than the one from compiled .proto file #340

Open
nodarai opened this issue Sep 18, 2022 · 3 comments
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

@nodarai
Copy link

nodarai commented Sep 18, 2022

The message built with proto-plus package produces slightly different proto schema. The difference is that ._default_package is added to the nested field's type_name.
It causes an error when trying to use that schema to write to GCP BigQuery's Storage API: 400 Invalid proto schema: BqMessage.proto: UserSchema.team: "._default_package.UserSchema.Team" is not defined

Environment details

  • Programming language: Python
  • OS: Manjaro with 5.19 kernel
  • Language runtime version: 3.8.12
  • Package version: 1.22.0

Steps to reproduce

  1. Create the user.proto file
syntax = "proto2";

message UserSchema {
  required string username = 1;
  required string email = 2;

  message Team { required string name = 1; };

  required Team team = 3;
}
  1. Compile it
protoc --python_out=. user.proto
  1. Run the code
import asyncio

import proto
from google.cloud.bigquery_storage_v1beta2.types.protobuf import ProtoSchema
from google.protobuf.descriptor_pb2 import DescriptorProto

from . import user_pb2


class UserSchema(proto.Message):
    class Team(proto.Message):
        name = proto.Field(proto.STRING, number=1)

    username = proto.Field(proto.STRING, number=1)
    email = proto.Field(proto.STRING, number=2)
    team = proto.Field(Team, number=3)


async def main():
    proto_descriptor = DescriptorProto()
    UserSchema.pb().DESCRIPTOR.CopyToProto(proto_descriptor)
    proto_schema = ProtoSchema(proto_descriptor=proto_descriptor)

proto_descriptor2 = DescriptorProto()
user_pb2.UserSchema.DESCRIPTOR.CopyToProto(proto_descriptor2)
proto_schema2 = ProtoSchema(proto_descriptor=proto_descriptor2)

    print(proto_schema)
    print(proto_schema2)


if __name__ == "__main__":
    asyncio.run(main())
  1. Compare the output's team field's type names
proto_descriptor {
  name: "UserSchema"
  field {
    name: "username"
    number: 1
    label: LABEL_OPTIONAL
    type: TYPE_STRING
  }
  field {
    name: "email"
    number: 2
    label: LABEL_OPTIONAL
    type: TYPE_STRING
  }
  field {
    name: "team"
    number: 3
    label: LABEL_OPTIONAL
    type: TYPE_MESSAGE
    type_name: "._default_package.UserSchema.Team"
  }
  nested_type {
    name: "Team"
    field {
      name: "name"
      number: 1
      label: LABEL_OPTIONAL
      type: TYPE_STRING
    }
    options {
    }
  }
  options {
  }
}
proto_descriptor {
  name: "UserSchema"
  field {
    name: "username"
    number: 1
    label: LABEL_REQUIRED
    type: TYPE_STRING
  }
  field {
    name: "email"
    number: 2
    label: LABEL_REQUIRED
    type: TYPE_STRING
  }
  field {
    name: "team"
    number: 3
    label: LABEL_REQUIRED
    type: TYPE_MESSAGE
    type_name: ".UserSchema.Team"
  }
  nested_type {
    name: "Team"
    field {
      name: "name"
      number: 1
      label: LABEL_REQUIRED
      type: TYPE_STRING
    }
  }
}
  1. type_name: "._default_package.UserSchema.Team" from proto.Message class and type_name: ".UserSchema.Team" from compiled .proto file
@nodarai nodarai added 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. labels Sep 18, 2022
@nodarai
Copy link
Author

nodarai commented Sep 19, 2022

Just found this:
https://github.com/googleapis/proto-plus-python/blob/main/proto/_package_info.py#L40

    # TODO: Revert to empty string as a package value after protobuf fix.
    # When package is empty, upb based protobuf fails with an
    # "TypeError: Couldn't build proto file into descriptor pool: invalid name: empty part ()' means"
    # during an attempt to add to descriptor pool.

Which is probably the reason for the above error.

@nodarai
Copy link
Author

nodarai commented Sep 20, 2022

Is the above mentioned issue being worked on?
Or is it at least in the roadmap?

@nodarai
Copy link
Author

nodarai commented Sep 29, 2022

The cause issue was fixed in protobuf version 4.21.6, so I have created a PR #341 to apply the fix to this package.
Meanwhile, before the PR is merged and released, to make things work with protobuf=4.21.6, adding the next line to the above code will fix the issue:

__protobuf__ = proto.module(package="")

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

No branches or pull requests

1 participant