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

proto3 optional feature #81

Open
Opa- opened this issue Nov 20, 2024 · 0 comments
Open

proto3 optional feature #81

Opa- opened this issue Nov 20, 2024 · 0 comments
Assignees

Comments

@Opa-
Copy link

Opa- commented Nov 20, 2024

Hello,

I'm currently converting some services to Temporal workers. These services already defines Protobuf files and sometimes rely on optional fields.

I'm getting warning message when using those proto messages. Here is a simple example :

syntax = "proto3";

package foo.v1;

import "temporal/v1/temporal.proto";

service FooService {
    option (temporal.v1.service) = {
        task_queue: "foo-queue"
    };

    rpc Foo (FooInput) returns (FooOutput) {
        option (temporal.v1.workflow) = {};
        option (temporal.v1.activity) = {
            start_to_close_timeout: {seconds: 2}
        };
    }
}

message FooInput {
    optional string bar = 1;
}

message FooOutput {
    optional string bar = 1;
}
WARN	plugin "protoc-gen-go_temporal" does not support required features.
  Feature "proto3 optional" is required by 1 file(s):
    foo/v1/foo.proto

The generated code do not work either as the generated UnmarshalCliFlagsToFooInput will consider the field to be oneof (here https://github.com/cludden/protoc-gen-go-temporal/blob/main/internal/plugin/cli.go#L944) and use unresolved references XBar and unresolved type FooInput_Bar :

if cmd.IsSet("bar") {
    hasValues = true
    result.XBar = &FooInput_Bar{Bar: cmd.String("bar")}
}

Resulting in the following error :

gen/foo/v1/foo_temporal.pb.go:1181:10: result.XBar undefined (type FooInput has no field or method XBar)
gen/foo/v1/foo_temporal.pb.go:1181:18: undefined: FooInput_Bar

If using proto2 there's not warning, the field is not considered as oneof (here https://github.com/cludden/protoc-gen-go-temporal/blob/main/internal/plugin/cli.go#L947) but the generated code do not work either as cmd.String is returning a string where the field is actually a *string.

if cmd.IsSet("bar") {
    hasValues = true
    result.Bar = cmd.String("bar")
}
gen/foo/v1/foo_temporal.pb.go:1181:16: cannot use cmd.String("bar") (value of type string) as *string value in assignment

Is there any technical reason/limitation or design philosophy not to support optional feature ? And if not would you think it would be possible to add support for it ?

Note that the issue seems to be only related to the generated CLI as buf with option cli-enabled=false do not generate any invalid code.

Thanks

@cludden cludden self-assigned this Nov 27, 2024
cludden added a commit that referenced this issue Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants