issuu / ocaml-protoc-plugin

ocaml-protoc-plugin
https://issuu.github.io/ocaml-protoc-plugin/
Other
48 stars 19 forks source link

Create rpcs as modules including method name #28

Closed Nymphium closed 1 year ago

Nymphium commented 3 years ago

This is PR for #27 .

The modified code generates echo.ml with examples/echo/echo.proto:

......
  module Echo = struct
    module Call = struct
      let name' () = "/echo.Echo/Call"
      module Request = Request
      module Response = Reply
    end
    let call =
    ( (module Request : Runtime'.Service.Message with type t = Request.t ),
      (module Reply : Runtime'.Service.Message with type t = Reply.t ) )
    let call' = (module Call : Runtime'.Service.Rpc with type Request.t = Request.t and type Response.t = Reply.t)
  end
......

name' says its method name in the same way as name' of messages. I leave "call" for compatibility. call' can be used with Ocaml_proto_plugin.Service.make_client_functions' (and make_service_funcrtions').

bobot commented 2 years ago

@andersfugmann Can we help in some way the merge of this MR?

bobot commented 2 years ago

@Nymphium https://github.com/bobot/ocaml-protoc-plugin/commits/streaming_description adds on top of your branch upstream mode of client and server. It also rename val name' by val name.

andersfugmann commented 1 year ago

Thanks for the PR, and sorry the long delay in replying.

  module Echo = struct
    module Call = struct
      let name' () = "/echo.Echo/Call"
      module Request = Request
      module Response = Reply
    end
    let call =
    ( (module Request : Runtime'.Service.Message with type t = Request.t ),
      (module Reply : Runtime'.Service.Message with type t = Reply.t ) )
    let call' = (module Call : Runtime'.Service.Rpc with type Request.t = Request.t and type Response.t = Reply.t)
  end

If I understand the proposed changes, the PR adds the module Call (copying your example above). If the function call' as an alias for the module Call needed?. If not, I gather that we also do not need the extra functions make_client_functions' and `make_service_functions', which would make the changes smaller.

While adding a new module, I don't think we need to repeat the existing mistake of adding val name': unit -> string, but just have val name : string (As we are not using recursive modules, we don't need to make name a function.

I have some comments on the PR itself, but In general I think this is the right approach.

andersfugmann commented 1 year ago

It would be nice to have some verification (tests) of the functionality - i.e. that the modules are created correctly, and contains correct names.

andersfugmann commented 1 year ago

Thinking of how to deprecate the old interface, and replace with this one; we could add a functor to the Service module to allow easy access to the serialization function. Something in the lines of:

module Make(U : Rpc) = struct
  let client_functions = U.Request.to_proto, U.Reply.from_proto
  let service_functions = U.Reply.to_proto, U.Request.from_proto
end

We can then mark the existing functions as deprecated as well as the generated Service function, but I think it belongs to a different PR.