Code review comment for lp:~jameinel/juju-core/reflect-only-facades

Revision history for this message
John A Meinel (jameinel) wrote :

Reviewers: mp+218764_code.launchpad.net,

Message:
Please take a look.

Description:
rpc: expose concrete types

This changes the RPC package a bit so that if you are declaring
RootMethod that returns 'interface{}' it can find the concrete type that
is being returned and expose the methods on that type (instead of
exposing 0 methods from interface{}).

This is a step along the way to supporting passing versions for Facades
in the "id" slot. We currently let you pass the value, but the concrete
type that is returned defines all of the Methods that could be
available. Which means that if you add a new Method in V2 of your API,
you would suddenly have that exposed in the V1 of your API. Similarly
you couldn't *actually* ever change the interface for a give method,
because then you couldn't call it one way for V1 and a different way for
V2.

There is a fair bit more to be done before we have actual API
versioning, but this is a step along that path and it seemed cohesive
enough to review on its own.

https://code.launchpad.net/~jameinel/juju-core/reflect-only-facades/+merge/218764

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/98090043/

Affected files (+290, -61 lines):
   A [revision details]
   M rpc/reflect_test.go
   M rpc/rpc_test.go
   M rpc/rpcreflect/type.go
   M rpc/rpcreflect/value.go
   M rpc/server.go

« Back to merge proposal