Have separate package for rpc per library/encoding?

[bufdev]

Right now, modules/rpc is basically modules/yarpcthrift, ie it's not really for a generic rpc module, it's specifically for yarpc using the thrift encoding. Of course, you could add other rpc libraries (grpc for example) and other encodings (protobuf or avro for example) in the same package, but then you're talking about a lot of dependencies that aren't used for anyone importing this package.

I'd like to see something like:

modules/yarpc modules/yarpc/thrift modules/yarpc/protobuf modules/grpc modules/grpc/protobuf

[bufdev]

@breerly

[HelloGrayson]

We won't need to expose another RPC framework besides YARPC because YARPC will do that for us.

Besides the point: I agree, we need to expose all the functionally YARPC makes possible, and right now that is artificially limited to 1 encoding. A good litmus test would be: if we add a new encoding, like proto for example, there should be no work required in UberFX to expose that, it should just work.

The config project we are working on now (search "config:" in YARPC-go PRs) should resolve this (or put us in the position where it's the next thing to resolve): since the whole rpc: key will be delegated to yarpc, you'll need to be able to use it to work with any encodings.

I feel less strongly about whether the config key is named rpc: or yarpc:, but I sort of prefer the explicitness in the latter.

[bufdev]

"We" = Uber, I'd love if FX was usable as a general framework for everyone, including those not tied to YARPC, and there are some places where do we close tie-ins, but this is one that's easy to make sure we don't have a tie-in.

[HelloGrayson]

In other words, I do think:

• the yarpc module should reside in the modules/yarpc package
• the functionality should be exposed in such a way that all encodings can be used
• the rpc: key should probably be renamed to the more explicit yarpc:

[bufdev]

Note that in https://github.com/uber-go/fx/pull/275, I'm removing default names for modules as it is proposed now, so the key will not longer be tied by default to rpc.

[bufdev]

Oh also, I agree with @breerly's above list

[bufdev]

I don't want to do this until we get some of the bigger PRs in, otherwise we will be in merge world

[yutongp]

renaming has been discussed in #267 @yichengq