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

Support optional annotation in proto3 files in generators #1278

Closed
johanbrandhorst opened this issue May 8, 2020 · 15 comments · Fixed by #1951
Closed

Support optional annotation in proto3 files in generators #1278

johanbrandhorst opened this issue May 8, 2020 · 15 comments · Fixed by #1951

Comments

@johanbrandhorst
Copy link
Collaborator

protoc v3.12.0 is going to add experimental support for the optional annotation to proto3 (see RC: https://github.com/protocolbuffers/protobuf/releases/tag/v3.12.0-rc1). We may need to update our generator(s) (protoc-gen-grpc-gateway, protoc-gen-swagger). Doc for adding this ability is here: https://github.com/protocolbuffers/protobuf/blob/v3.12.0-rc1/docs/implementing_proto3_presence.md.

We're probably only going to add this to v2. It might be an opportunity to explore rewriting the generators with https://pkg.go.dev/google.golang.org/protobuf/compiler/protogen?tab=doc.

@Goobs Goobs mentioned this issue May 28, 2020
13 tasks
@stale
Copy link

stale bot commented Jul 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@buzzdan
Copy link

buzzdan commented Oct 6, 2020

i've upgraded to protoc v3.13.0
when running it on a file with an optional field i get this message:
entities/v1/account.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-grpc-gateway hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--grpc-gateway_out: make: *** [protoc] Error 1

@johanbrandhorst
Copy link
Collaborator Author

What version of the generator are you using? I think we might support this on v2 after #1668. Could you try https://github.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.0.0-beta.5?

CC @adambabik

@buzzdan
Copy link

buzzdan commented Oct 7, 2020

@johanbrandhorst

// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// 	protoc-gen-go v1.25.0
// 	protoc        v3.13.0

@johanbrandhorst
Copy link
Collaborator Author

I mean, what version of protoc-gen-grpc-gateway? Those version headers don't help me I'm afraid. I linked you a release, could you try generating with that release?

@buzzdan
Copy link

buzzdan commented Oct 7, 2020

well i installed it via go get ... so i guess the latest version published

$ protoc-gen-grpc-gateway -version
Version dev, commit unknown, built at unknown

any other way to know the version ?

btw i did had the experiemetal flag on
this is my command:

"${PROTOC_BIN}" --proto_path="${PROTO_FILES_DIR}" --proto_path=pkg/internal/third_party --grpc-gateway_out=logtostderr=true,paths=source_relative:pkg/internal/generated/grpcgateway --swagger_out=logtostderr=true:pkg/internal/generated/swagger --go-grpc_out=${OPTS},gen_unstable_server_interfaces=true,paths=source_relative:pkg/internal/generated/grpcgateway --go_out=${OPTS},paths=source_relative:pkg/internal/generated/grpcgateway --experimental_allow_proto3_optional "${file}";

@buzzdan
Copy link

buzzdan commented Oct 7, 2020

@johanbrandhorst
about the beta i'll have to find some time for it later
i'll keep you posted once i have something

glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 22, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 22, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 23, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Nov 25, 2020
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
@0xKiwi
Copy link

0xKiwi commented Dec 13, 2020

Hello! Any update on this? I'm trying to use the proto-gen-grpc-gateway with optional protos and I get:

eth/v1/beacon_chain_service.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-grpc-gateway hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--grpc-gateway_out: 

Pretty sure I'm using grpc-gateway/v2, in my go.mod:

	github.com/grpc-ecosystem/grpc-gateway/v2 v2.0.1 // indirect

@johanbrandhorst
Copy link
Collaborator Author

We can't add this support yet, but we're also not sure exactly what it should mean for the generator. See #1834 (comment) and #1834 (comment). What would you expect the effect to be on the generated files?

@0xKiwi
Copy link

0xKiwi commented Dec 15, 2020

Hello @johanbrandhorst, I am using optional in my protos and they are required for another part of my application. Although grpc-gateway not supporting optional fields is problematic because the current code generator fails to compile when it encounters an optional field.

Maybe the functionality shouldn't be changed, but can the generator be changed to at least compile the files properly? Maybe ignore the optional entirely?

Also, maybe #1871 is related as well, since the behavior of omitempty is very similar to what is expected of optional, for the endpoint to be able to tell whether a field is passed in or not, as some types (like uint64) are hard to distinct from zero-value, or just assigned as 0.

@0xKiwi
Copy link

0xKiwi commented Jan 14, 2021

Hello! Bumping this to see if there has been any progress!

Adding support for optionals isn't really required for this I think, although it would be great if optional protos didn't break compilation! Thank you!

@johanbrandhorst
Copy link
Collaborator Author

It definitely shouldn't be breaking compilation, what errors are you seeing? The v2 generator is now built on top of protogen which should support this.

@vincentvaroquauxads
Copy link

Hello !
I have the same issue.

I installed protoc-gen-go & protoc-gen-grpc-gateway as follows:

> go get -u github.com/golang/protobuf/protoc-gen-go
go: found github.com/golang/protobuf/protoc-gen-go in github.com/golang/protobuf v1.4.3
go: google.golang.org/protobuf upgrade => v1.25.0

> go get -u github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway
go: found github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway in github.com/grpc-ecosystem/grpc-gateway/v2 v2.1.0
go: gopkg.in/yaml.v2 upgrade => v2.4.0
go: google.golang.org/protobuf upgrade => v1.25.0
go: github.com/golang/protobuf upgrade => v1.4.3
go: google.golang.org/genproto upgrade => v0.0.0-20210203152818-3206188e46ba
go: downloading google.golang.org/genproto v0.0.0-20210203152818-3206188e46ba

When I generate, I have the following error:
.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-grpc-gateway hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--grpc-gateway_out:

Anyway, I'm not sure I provide any new information...

@adambabik
Copy link
Collaborator

I think we need to explicitly set that optional in proto3 is supported like this https://github.com/protocolbuffers/protobuf-go/blob/ac374a2335472e7ef5b5c28e72c3060121b8d1ed/cmd/protoc-gen-go/main.go#L49 in

}.Run(func(gen *protogen.Plugin) error {
reg := descriptor.NewRegistry()
err := applyFlags(reg)
if err != nil {
return err
}
generator := gengateway.New(reg, *useRequestContext, *registerFuncSuffix, *allowPatchFeature, *standalone)
glog.V(1).Infof("Parsing code generator request")
if err := reg.LoadFromPlugin(gen); err != nil {
return err
}
unboundHTTPRules := reg.UnboundExternalHTTPRules()
if len(unboundHTTPRules) != 0 {
return fmt.Errorf("HTTP rules without a matching selector: %s", strings.Join(unboundHTTPRules, ", "))
}
var targets []*descriptor.File
for _, target := range gen.Request.FileToGenerate {
f, err := reg.LookupFile(target)
if err != nil {
return err
}
targets = append(targets, f)
}
files, err := generator.Generate(targets)
for _, f := range files {
glog.V(1).Infof("NewGeneratedFile %q in %s", f.GetName(), f.GoPkg)
genFile := gen.NewGeneratedFile(f.GetName(), protogen.GoImportPath(f.GoPkg.Path))
if _, err := genFile.Write([]byte(f.GetContent())); err != nil {
return err
}
}
glog.V(1).Info("Processed code generator request")
return err
})
. I can test it tomorrow.

@adambabik
Copy link
Collaborator

It was a bit more complicated but #1951 should fix it. I added an example with an optional field. It's actually two step process where --experimental_allow_proto3_optional needs to be provided for protoc but each individual plugin needs to report that it supports the optional field feature as well.

glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Feb 20, 2021
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Feb 20, 2021
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
glerchundi added a commit to glerchundi/grpc-gateway that referenced this issue Feb 20, 2021
Let `protoc` know that these generators support optional keywords. This
is by no means an implementation in `grpc-gateway` of the optional aware
generated code, it's just shielding of being considered as blockers in
code generation workflows.

Closes grpc-ecosystem#1278
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment