googleforgames / agones

Dedicated Game Server Hosting and Scaling for Multiplayer Games on Kubernetes
https://agones.dev
Apache License 2.0
6k stars 791 forks source link

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

Closed steven-supersolid closed 4 years ago

steven-supersolid commented 4 years ago

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 commented 4 years ago

/assign steven-supersolid

steven-supersolid commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

Have now filed an issue https://github.com/grpc/grpc-node/issues/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.