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

[Feature] Support import external proto in goctl. #4268

Open
chaozwn opened this issue Jul 23, 2024 · 5 comments
Open

[Feature] Support import external proto in goctl. #4268

chaozwn opened this issue Jul 23, 2024 · 5 comments
Assignees
Labels
area/goctl Categorizes issue or PR as related to goctl.

Comments

@chaozwn
Copy link

chaozwn commented Jul 23, 2024

Is your feature request related to a problem? Please describe.
In some scenarios, we may need to perform automatic validation of rpc parameters. for example, protovalidate-go library is introduced to automatically validate req.

  1. This is the proto that I generated. It contains an external proto file and a new validation syntax.
    image
    Next, I can verify req by directly generating the code automatically.
    image

Describe the solution you'd like

  1. In fact, I have tested that it is now perfectly possible to use buf-cli instead of protoc to generate go files for grpc and proto.
    The following is my solution case.

buf.gen.yaml

version: v2

plugins:
  - remote: buf.build/grpc/go
    out: ../
  - remote: buf.build/protocolbuffers/go
    out: ../
  - remote: buf.build/bufbuild/validate-go
    out: ../

buf.yaml

version: v2

name: buf.build/tutorials/lint
breaking:
  use:
    - FILE
lint:
  use:
    - DEFAULT
  except:
    - SERVICE_SUFFIX
    - FIELD_LOWER_SNAKE_CASE
    - PACKAGE_LOWER_SNAKE_CASE
    - PACKAGE_SAME_SWIFT_PREFIX
    - PACKAGE_VERSION_SUFFIX
    - ENUM_NO_ALLOW_ALIAS
    - BASIC
    - RPC_REQUEST_STANDARD_NAME
    - RPC_RESPONSE_STANDARD_NAME

deps:
  - buf.build/bufbuild/protovalidate

update dep

buf dep update

generate grpc & proto file

buf generate --path ./app/todo/cmd/rpc/pb/todo.proto --output ./app/todo/cmd/rpc/pb

image

In summary, we can get exactly the same grpc file and pb.go file with proto introduced.

But now there is a very fatal problem, I have import other proto files in proto early, resulting in the system's built-in generation function directly reported an error.

image

Describe alternatives you've considered

So I wonder if this syntax can be supported. goctl rpc buf xx.proto --xxx. If you are OK with my idea, I can try to submit a pr along these lines.

Additional context
Add any other context or screenshots about the feature request here.

@kesonan
Copy link
Collaborator

kesonan commented Jul 25, 2024

maybe you can try with -I $external_path, goctl rpc protoc $proto ... -I $external_path --zrpc_out $dir --style=goZero --home $home -m

@chaozwn
Copy link
Author

chaozwn commented Jul 25, 2024

maybe you can try with -I $external_path, goctl rpc protoc $proto ... -I $external_path --zrpc_out $dir --style=goZero --home $home -m

first this directive has no way of taking effect, and the external proto code is not compiled by default

@kevwan kevwan added the area/goctl Categorizes issue or PR as related to goctl. label Aug 4, 2024
@fireice009
Copy link

I make it work. You also need -I for the path of .proto file. e.g. goctl rpc protoc -I ../../common/protobuf/ -I api/proto api/proto/user.proto --go_out=api --go-grpc_out=api --zrpc_out=api --client=true -m

@fireice009
Copy link

fireice009 commented Oct 22, 2024

I get a issue on gateway with importing validate.proto.

  1. Execute commands and got 3 *.pb files.
	protoc -I ../../common/protobuf/ -I api/proto --descriptor_set_out=api/asset/user.pb api/proto/user.proto
	protoc -I ../../common/protobuf/ --descriptor_set_out=api/asset/validate.pb ../../common/protobuf/validate.proto
	protoc -I ../../common/protobuf/google/protobuf --descriptor_set_out=api/asset/google/protobuf/descriptor.pb ../../common/protobuf/google/protobuf/descriptor.proto   # try 1
	protoc -I ../../common/protobuf/google/protobuf --descriptor_set_out=api/asset/descriptor.pb ../../common/protobuf/google/protobuf/descriptor.proto   # try 2
  1. gateway config
      ProtoSets:
        - asset/user.pb
        - asset/validate.pb
        - asset/google/protobuf/descriptor.pb  # try 1
        - asset/descriptor.pb # try 2
  1. Run gateway service, and get the error msg no descriptor found for "google/protobuf/descriptor.proto"

@fireice009
Copy link

I get a issue on gateway with importing validate.proto.

  1. Execute commands and got 3 *.pb files.
	protoc -I ../../common/protobuf/ -I api/proto --descriptor_set_out=api/asset/user.pb api/proto/user.proto
	protoc -I ../../common/protobuf/ --descriptor_set_out=api/asset/validate.pb ../../common/protobuf/validate.proto
	protoc -I ../../common/protobuf/google/protobuf --descriptor_set_out=api/asset/google/protobuf/descriptor.pb ../../common/protobuf/google/protobuf/descriptor.proto   # try 1
	protoc -I ../../common/protobuf/google/protobuf --descriptor_set_out=api/asset/descriptor.pb ../../common/protobuf/google/protobuf/descriptor.proto   # try 2
  1. gateway config
      ProtoSets:
        - asset/user.pb
        - asset/validate.pb
        - asset/google/protobuf/descriptor.pb  # try 1
        - asset/descriptor.pb # try 2
  1. Run gateway service, and get the error msg no descriptor found for "google/protobuf/descriptor.proto"

forget it. it work by adding --include_imports

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/goctl Categorizes issue or PR as related to goctl.
Projects
None yet
Development

No branches or pull requests

4 participants