elixir-grpc / grpc

An Elixir implementation of gRPC
https://hex.pm/packages/grpc
Apache License 2.0
1.4k stars 214 forks source link

Duplicate encode/decode protobuf message modules #232

Closed wingyplus closed 2 years ago

wingyplus commented 2 years ago

Currently, we have 2 modules that do encode/decode protobuf message:

The first one is GRPC.Codec.Proto, the usage in code base is:

$ ag 'GRPC.Codec.Proto'
lib/grpc/channel.ex
38:            codec: GRPC.Codec.Proto,

lib/grpc/client/stream.ex
46:            codec: GRPC.Codec.Proto,

lib/grpc/stub.ex
156:    codec = Keyword.get(opts, :codec, GRPC.Codec.Proto)

lib/grpc/server.ex
47:      codecs = opts[:codecs] || [GRPC.Codec.Proto]

lib/grpc/codec/proto.ex
1:defmodule GRPC.Codec.Proto do

lib/grpc/transport/http2.ex
61:    if codec == GRPC.Codec.Proto do

lib/grpc/server/stream.ex
49:            codec: GRPC.Codec.Proto,

test/grpc/integration/codec_test.exs
23:      codecs: [GRPC.Codec.Proto, GRPC.Codec.Erlpack]
44:      {:ok, reply} = channel |> HelloErlpackStub.say_hello(req, codec: GRPC.Codec.Proto)

And the second one is GRPC.Message.Protobuf which do similar thing but accept mod and struct separately. It's no usage in the code base but just test:

$ ag 'GRPC.Message.Protobuf'
lib/grpc/message/protobuf.ex
1:defmodule GRPC.Message.Protobuf do

test/grpc/message/protobuf_test.exs
1:defmodule GRPC.Message.ProtobufTest do
22:             GRPC.Message.Protobuf.encode(Helloworld.HelloRequest, request)
28:    assert ^request = GRPC.Message.Protobuf.decode(Helloworld.HelloRequest, msg)
35:    assert ^request = GRPC.Message.Protobuf.decode(Helloworld.HelloReply, msg)

I purpose to remove GRPC.Message.Protobuf since there's no usage in our code base. I can open a PR to fix this issue. :)

polvalente commented 2 years ago

Is it used in tests? Perhaps we can just remove GRPC.Message.Protobuf and replace with the other one

wingyplus commented 2 years ago

Is it used in tests? Perhaps we can just remove GRPC.Message.Protobuf and replace with the other one

See only its tests, not other place. :(