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

Allow FileDescriptorSets in place of individual .proto files #1071

Closed
tatemz opened this issue Oct 15, 2020 · 5 comments
Closed

Allow FileDescriptorSets in place of individual .proto files #1071

tatemz opened this issue Oct 15, 2020 · 5 comments
Labels

Comments

@tatemz
Copy link
Contributor

tatemz commented Oct 15, 2020

Most protocol buffer and gRPC projects that I have seen involve many proto files that typically exist outside of the client codebase. These large sets of proto files can be compiled into a file descriptor set:

protoc -I . --descriptor_set_out=descriptor-set.bin foo.proto

These descriptor sets can then be consumed during the protoc code generation process with --descriptor_set_in:

mkdir codegen_dir
protoc --descriptor_set_in=descriptor-set.bin --java_out=codegen_dir foo.proto

Rather than having to include many *.proto files, it would be cleaner (IMHO) if we could use a descriptor set. Example:

sources:
  - name: MyGrpcApi
    handler:
      grpc:
        endpoint: localhost:50051
        protoFilePath: foo.proto # this will still be needed to identify which proto definition in the descriptor set should be used.
        fileDescriptorSet: descriptor-set.bin
        serviceName: Example # maybe this isn't needed
        packageName: io.xtech.example # maybe this isn't needed
@tatemz
Copy link
Contributor Author

tatemz commented Oct 15, 2020

Adding a possible scenario to this feature request, it could be feasible for a single descriptor set to be passed in and all of the services contained in it automatically added without having to specify the descriptors within it. 🤔

@ardatan
Copy link
Owner

ardatan commented Oct 20, 2020

Sounds cool! Do you think you can create a PR so we can merge it and release a new version with this feature?

@tatemz
Copy link
Contributor Author

tatemz commented Oct 20, 2020

I'll work on it. Been reading up on how to do it and there might be a potential showstopper.

In order to convert a FileDescriptorSet into a Root object (used by graphql-mesh), we'd need to use the descriptor extension.

Then we'd need to set the packageDefinition variable using createPackageDefinition from grpc/grpc-node. Unfortunately, that function is not exported and grpc/grpc-node#550 was closed.

Looks like others have opened up issues on protobuf.js (protobufjs/protobuf.js#1117). I'll keep digging and code up a sample PR if I can.

@tatemz
Copy link
Contributor Author

tatemz commented Oct 21, 2020

Okay, I shifted gears and am starting with reflection support. I did fiddle with using statically generated descriptor sets, but was having some trouble with decoding them.

Check out #1086. Once reflection is working. It shouldn't be too hard to also support file descriptor sets.

@theguild-bot theguild-bot mentioned this issue Aug 11, 2022
@ardatan
Copy link
Owner

ardatan commented Mar 31, 2023

@ardatan ardatan closed this as completed Mar 31, 2023
@theguild-bot theguild-bot mentioned this issue Sep 28, 2023
This was referenced Apr 30, 2024
This was referenced May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants