stream requests and response support in rpc

[IgorKarymov]

Hi, Thomas! Thanks for the development of such nice library!
I'm thinking about implementing of grpc erlang server and client on the base of it.
At the current moment, I'm on very early 'research' stage.
Could you please add stream option support to parser, to make it possible to compile base grpc service example like this one: https://github.com/grpc/grpc/blob/release-0_14/examples/protos/route_guide.proto
It also will require adding some kind of 'stream' indication to find_rpc_def/2 return value.

[tomas-abrahamsson]

Thanks! Parsing the stream option elements ought to be doable, will take a look at it

[tomas-abrahamsson]

On the branch rpc-stream, there's a possibility to get info on the stream indications in the rpc defintions, maybe you'd like to try it out, and if you find it useful, I'll merge and push it.

Some implementation details:

• I've kept the old data format and the old api intact:
  • This means (a) that if one compiles to_proto_defs, then the old {{service,Name},Rpcs} still have the same format, but there is also the full info available as: {{service2,Name},[#gpb_rpc2{...}]}.
  • It also means (b) that the api functions in the generated code, still return data on the same format, but there are also new api functions generated: M:get_service2_def/1, M:find_rpc2_def/2, M:fetch_rpc2_def/2, these return data containing #gpb_rpc2{} records.
• The #gpb_rpc2{} record has two new fields, input_stream :: boolean() and output_stream :: boolean(). I hope this is fine, and useful? (I'm not very acquainted with the services or rpcs, so had to read up a little) It also has a new opts :: [term()] field for any options, should they be needed.
• The above means I've kept all since-previously existing records unchanged, because I've found changing them often leads to odd and difficult-to-track-down run-time issues if some part of a project hasn't been re-compiled with new record definitions, and some other part uses the new record definitions. So I did a version number in the record name instead, and a version number in the api functions.
  • However, if you're running with the maps option, or the defs_as_proplists option, then I reasoned that---unlike records---these two data structures can be extended without existing code breaking, or even need recompilation, so with any of these options, the new fields are present already in the return value of the non-2-named functions.
• I've made no effort to parse the proto2 stream definitions. It seems these are not valid in proto3, so I'm guessing they're kind of deprecated, and that you don't need them, is that ok?

I think especially the part with how the data format is still backwards compatible, is the way I'd like it to be, ie not changing existing records.

[IgorKarymov]

Thank you for so quick reaction. Few things here:

• old functions find_rpc_def/2 and get_service2_def/1 now can return rpc definitions with streams but with streams omitted, not sure if it's ok, because new rpc functions will try to mimic old one, so in practice it can lead to not compatible services (for example old clients will try to call server like ordinal rpc service, but server will expect a stream on input or will reply with stream of messages as output).
• I didn't find any erlang client or server realisation of google rpc format (probably piqi can count as proto2, but it using it's own proto parser), so maybe we can be more courageous in breaking backwards compatibility (but I believe you have more information than me)
  • google decide to call it 'grpc' so maybe it's a good idea to call it grpc instead of rpc2 as well.
  • I will start with the realisation of cowboy based grpc server shortly and will be able to send your some additional feedback. So we can keep it in branch for a while if you are not sure about details. I would like to implement a client too, but unfortunately, there is no trustable http2 client library around.

[tomas-abrahamsson]

Ah, yes, I think you're right, that it's a bit cautios

edit sorry, hit comment too early

[tomas-abrahamsson]

Ah, yes, I think you're right that it's a bit over-cautious to keep the #?gpb_rpc{} record unchanged, not to mention that it'll make everything a bit more difficult to understand and keep track of. I'll rework.

(meant to say over-cautious in last post, not cautious, when accidentally hitting comment)

[tomas-abrahamsson]

New version pushed. I'll probably merge this shortly, and if you discover anything more to fix or add or so, we can take it from there

[IgorKarymov]

Thanks! I will test new version as soon as possible. Actually, I already have a near to working version. Just stuck a little bit, with cowboy support of trailers headers ninenines/cowboy#1017
I also want to generate custom behaviour for each rpc service based on provided by gpb information (probably will have to create another rebar plugin that will depend on gpb).

[lrascao]

@IgorKarymov, let me know what are your plugin requirements, i'd be happy to take a look at it

[tomas-abrahamsson]

I merged the rpc-stream branch and just pushed 3.25.1, so closing this. Should you find anything more is needed, either re-open, or just open another issue.