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

Switch Node.js SDK grpc dependency to grpc-js #1489

Closed
steven-supersolid opened this issue Apr 23, 2020 · 4 comments · Fixed by #1529
Closed

Switch Node.js SDK grpc dependency to grpc-js #1489

steven-supersolid opened this issue Apr 23, 2020 · 4 comments · Fixed by #1529
Assignees
Labels
kind/feature New features for Agones
Milestone

Comments

@steven-supersolid
Copy link
Collaborator

The original node.js grpc library has been replaced by a new implementation and after a year the original library will be deprecated. The recommendation from the gRPC team is to update.
https://grpc.io/blog/grpc-js-1.0/

We should try the new library to see if there are any compatibility issues and then log with the gRPC project. Also will be interesting to see if the new library fixes #1299, although this bug has not yet been reproduced in our tests.

We can safely remain on the old library if there are problems moving over.

Instructions to update look straightforward: https://www.npmjs.com/package/@grpc/grpc-js

@steven-supersolid steven-supersolid added the kind/feature New features for Agones label Apr 23, 2020
@steven-supersolid
Copy link
Collaborator Author

/assign steven-supersolid

@steven-supersolid
Copy link
Collaborator Author

I took a look at this but need to follow a couple of lines of investigation:

When I modify this command inside build/build-sdk-images/node/gen.sh

protoc -I ${googleapis} -I ${sdk} --grpc_out=generate_package_definition:minimum_node_version=12:./sdks/nodejs/lib --plugin=protoc-gen-grpc=`which grpc_node_plugin` sdk.proto

The executed version is:

protoc -I /go/src/agones.dev/agones/proto/googleapis -I /go/src/agones.dev/agones/proto/sdk --grpc_out=generate_package_definition:minimum_node_version=12:./sdks/nodejs/lib --plugin=protoc-gen-grpc=/usr/local/bin/grpc_node_plugin sdk.proto

I receive the error message

--grpc_out: sdk.proto: Unknown parameter: generate_package_definition

@markmandel
Copy link
Member

So it seems grpc-tools refers to https://www.npmjs.com/package/grpc-tools rather than protoc.

Might be worth filing an issue with the team for guidance on what to do when loading from protoc generated files (rather than grpc-tools).

@steven-supersolid
Copy link
Collaborator Author

I think grpc-tools just wraps protoc and the node plugin https://github.com/grpc/grpc-node#grpc-tools

Have now filed an issue grpc/grpc-node#1408

Should also be possible to install grpc-tools to see if it works but currently unsure how this would fit into the existing scripts.

Another option we may have in JS is not to use static codegen as the proto files can be loaded directly, however would have to copy the proto files and dependencies protos to the appropriate folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features for Agones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants