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 of protoset binary format? #1117

Open
c0b opened this issue Sep 27, 2018 · 5 comments
Open

support of protoset binary format? #1117

c0b opened this issue Sep 27, 2018 · 5 comments

Comments

@c0b
Copy link

c0b commented Sep 27, 2018

from grpc/grpc-node#556 wonder is the protoset binary format already supported? or can be added?

I have searched the keyword of grpc + nodejs + protoset on google and within this repo, nothing found so far

protobuf.load("awesome.protoset", function(err, root) {
  ...
}

I see the compiled protoset binary format is a good solution to bundle many related *.proto files into a single one file, it's pretty well supported by protoc compiler, and in Go language pkg github.com/golang/protobuf, and tools like grpcurl
https://github.com/fullstorydev/grpcurl#protoset-files
https://github.com/golang/protobuf/tree/master/protoc-gen-go/descriptor

$ protoc --help
Usage: protoc [OPTION] PROTO_FILES
Parse PROTO_FILES and generate output based on the options given:
  -IPATH, --proto_path=PATH   Specify the directory in which to search for
                              imports.  May be specified multiple times;
                              directories will be searched in order.  If not
                              given, the current working directory is used.

  -oFILE,                     Writes a FileDescriptorSet (a protocol buffer,
    --descriptor_set_out=FILE defined in descriptor.proto) containing all of
                              the input files to FILE.
@c0b
Copy link
Author

c0b commented Oct 26, 2018

hello?

@dcodeIO
Copy link
Member

dcodeIO commented Oct 26, 2018

Looked over protoset pretty quick, appears to be binary encoded file descriptor sets. There is no first-class support, but there's an extension that might be useful: https://github.com/dcodeIO/protobuf.js/tree/master/ext/descriptor

@c0b
Copy link
Author

c0b commented Dec 4, 2018

from grpc/grpc-node#550 (comment) I want to report a problem not sure which side can resolve it better?

because most grpc uses this require('@grpc/proto-loader') package's load or loadSync and that's the only exported methods, and it seems the authors of @grpc/proto-loader are reluctant to export more, the load or loadSync method's problem is they do load plain text *.proto files only,

to make it work with the require("protobufjs/ext/descriptor") I have to export the createPackageDefinition method from this module, but they're not willing to do so, so my temporary solution is to keep a copy its compiled code, and change createPackageDefinition method to be exported, I name it as loader.js and uses in my project require("./loader.js").createPackageDefinition(root, ...)

source code is mainly in this ts file

https://github.com/grpc/grpc-node/blob/master/packages/proto-loader/src/index.ts

example code from https://github.com/grpc/grpc/blob/master/examples/node/dynamic_codegen/greeter_client.js

var protoLoader = require('@grpc/proto-loader');
var packageDefinition = protoLoader.loadSync(
    PROTO_PATH,
    { keepCase: true, longs: String, enums: String, defaults: true, oneofs: true, });
var hello_proto = grpc.loadPackageDefinition(packageDefinition).helloworld;

keep a private copy is not a good solution, hope either side can have a better one

@tatemz
Copy link

tatemz commented Oct 20, 2020

+1 if file descriptor sets are a supported feature of protoc, then we will need the ability to create PackageDefinition objects using them.

keep a private copy is not a good solution, hope either side can have a better one

@tatemz
Copy link

tatemz commented Oct 27, 2020

@dcodeIO or @nicolasnoble Do you have time to look at the linked graphql-mesh issue (1086) and offer advice?
I would love the comments by @c0b to be addressed as I am in the same predicament.
It seems the path forward on this is to fork this project.

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

No branches or pull requests

4 participants