Skip to content

proto3 support for optional #229

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

Closed
SoerenBusse opened this issue Apr 7, 2021 · 13 comments
Closed

proto3 support for optional #229

SoerenBusse opened this issue Apr 7, 2021 · 13 comments

Comments

@SoerenBusse
Copy link

Proto3 supports the optional keyword. All libraries supporting this keyword needs to indicate this explicitly. Otherwise the complication will fail with is a proto3 file that contains optional fields, but code generator protoc-gen-python_betterproto hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.

Would it be possible to enable proto3 optional?

See this issue for more detail: vmagamedov/grpclib#125

@nat-n
Copy link
Collaborator

nat-n commented Apr 7, 2021

Thanks for bringing this up. Looks like this was quite recently added as a fully supported feature: https://github.com/protocolbuffers/protobuf/releases/tag/v3.15.0

This is kinda of a significant change to proto3 semantics, and will take a bit of thought about how to best adapt the API to support it. It doesn't look like the official proto3 guide has been updated to reflect this yet, but I found this guide which looks highly relevant: https://chromium.googlesource.com/external/github.com/protocolbuffers/protobuf/+/refs/heads/master/docs/implementing_proto3_presence.md

Updating the Message metadata model to support this should allow the removal of this hack in the plugin:

def monkey_patch_oneof_index():

@nat-n nat-n added this to the 2.0.0 milestone May 4, 2021
@marianhlavac
Copy link

Is there any progess on this? It's currently very important feature for us, and we're forced to use official python protoc plugin for now.

@roblabla
Copy link
Contributor

Hello, this is also very important for us. Is there any plan/timeline for this feature? What can we do to help speed up the implementation?

@kalzoo
Copy link
Collaborator

kalzoo commented Jul 10, 2021

My comment on #35 would have made more sense here - the quick-fix might be to just treat optional fields as single-variant oneof to unblock parsing definitions which include it.

@jdlourenco
Copy link

Are there any news on this?

@Gobot1234
Copy link
Collaborator

Yes this is nearly ready but there is an issue with the fields recursing infinitely #281

@jdlourenco
Copy link

@Gobot1234 are tests only failing on the pipeline? I ran poe test and poetry run pytest tests/ locally and they ran successfully.

@Gobot1234
Copy link
Collaborator

I'm not entirely sure what's up with it, @kalzoo is the person you should be asking if you'd be willing to do some debugging and provide a fix for the failing tests it will get merged and then beta 4 can be shipped with support for this.

@Gobot1234
Copy link
Collaborator

v.2.0.0b4 has just been released and includes support for this.

@atulsriv
Copy link

This is gonna be a long shot, but I am still seeing this error. I am on version of v.2.0.0b6, I have libprotoc 3.21.9. It should be working for me, but I still run into this error. been stuck for a few days, so if anyone has any ideas please lmk thanks

@seansica
Copy link

Issue still occurring.

Error:

src/foo/bar/message.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-python_betterproto hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.

pyproject.yaml:

protobuf = "^5.29.2"
betterproto = {extras = ["compiler"], version = "^1.2.5"}
❯ poetry run python -m pip list | grep proto
betterproto        1.2.5
mypy-protobuf      3.6.0
protobuf           5.29.2
types-protobuf     5.29.1.20241207
❯ poetry run python --version
Python 3.11.9

Relevant snippet of .proto file:

message Foo {
  // ..
  optional string foo = 45;
  // ...
  optional bool bar = 49;
}

@Gobot1234
Copy link
Collaborator

Use 2.0.0b6 please

@seansica
Copy link

Use 2.0.0b6 please

Thanks, that appears to work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants