api/uniter: Skeleton of remaining calls/objects

api/uniter: Skeleton of remaining calls/objects This adds all needed modules for the complete uniter client-side API, except for the RelationUnitsWatcher, which will come later. No tests, almost no implementation, just TODOs and method signatures to help with whoever picks it up while I'm away. As a follow-up I'll start implementing the rest of the uniter.Unit calls. https://code.launchpad.net/~dimitern/juju-core/107-api-uniter-skeleton/+merge/180893 (do not edit description out of merge proposal)

[dimitern, 2013-08-19 15:12:06]

Please take a look.

[fwereade, 2013-08-19 15:28:11]

LGTM with a few additional notes https://codereview.appspot.com/13107043/diff/1/state/api/uniter/endpoint.go File state/api/uniter/endpoint.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/endpoint.go#n... state/api/uniter/endpoint.go:12: // the uniter worker. Maybe incorporate this comment in the one below? https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relation.go File state/api/uniter/relation.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relation.go#n... state/api/uniter/relation.go:36: // TODO: Convert the relation tag to id and return it. This is inaccurate, I think? IIRC we agreed that tag had to be derived from key? https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relationunit.go File state/api/uniter/relationunit.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relationunit.... state/api/uniter/relationunit.go:57: var ErrCannotEnterScopeYet = errors.New("cannot enter scope yet: non-alive subordinate unit has not been removed") Don't these errors need to be in params? https://codereview.appspot.com/13107043/diff/1/state/api/uniter/settings.go File state/api/uniter/settings.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/settings.go#n... state/api/uniter/settings.go:24: func (s *Settings) Set(key string, value interface{}) { This should be a string, I think, so please note that even if we defer the actual change. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go File state/api/uniter/unit.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:156: // an error as well, because it needs to make an API call. Note that we might be able to get away with dropping this... https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:163: func (u *Unit) SetPublicAddress(address string) (err error) { and this https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:173: func (u *Unit) PrivateAddress() (string, bool, error) { and this https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:179: func (u *Unit) SetPrivateAddress(address string) error { and this https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:187: // TODO: Call Uniter.OpenPort() and that this should really be talking to a machine, not a unit, and so there's room for additional smarts here... https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:194: // TODO: Call Uniter.ClosePort() and here

[dimitern, 2013-08-19 19:49:49]

Please take a look. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/endpoint.go File state/api/uniter/endpoint.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/endpoint.go#n... state/api/uniter/endpoint.go:12: // the uniter worker. On 2013/08/19 15:28:12, fwereade wrote: > Maybe incorporate this comment in the one below? Done. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relation.go File state/api/uniter/relation.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relation.go#n... state/api/uniter/relation.go:36: // TODO: Convert the relation tag to id and return it. On 2013/08/19 15:28:12, fwereade wrote: > This is inaccurate, I think? IIRC we agreed that tag had to be derived from key? Well, yes and no :) Currently, it's accurate because relation tags are still derived from the id, rather than the key. And yes, this will change once we start using keys in relation tags. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relationunit.go File state/api/uniter/relationunit.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/relationunit.... state/api/uniter/relationunit.go:57: var ErrCannotEnterScopeYet = errors.New("cannot enter scope yet: non-alive subordinate unit has not been removed") On 2013/08/19 15:28:12, fwereade wrote: > Don't these errors need to be in params? Actually there are already such Code* constants defined in api/params/apierrors.go. Added a TODO to use them and do the error translation properly. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/settings.go File state/api/uniter/settings.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/settings.go#n... state/api/uniter/settings.go:24: func (s *Settings) Set(key string, value interface{}) { On 2013/08/19 15:28:12, fwereade wrote: > This should be a string, I think, so please note that even if we defer the > actual change. Good point, will add a TODO about it. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go File state/api/uniter/unit.go (right): https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:156: // an error as well, because it needs to make an API call. On 2013/08/19 15:28:12, fwereade wrote: > Note that we might be able to get away with dropping this... Yes, but not for now at least. I'll add a TODO about it. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:163: func (u *Unit) SetPublicAddress(address string) (err error) { On 2013/08/19 15:28:12, fwereade wrote: > and this I'll add a TODO. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:173: func (u *Unit) PrivateAddress() (string, bool, error) { On 2013/08/19 15:28:12, fwereade wrote: > and this same https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:179: func (u *Unit) SetPrivateAddress(address string) error { On 2013/08/19 15:28:12, fwereade wrote: > and this same https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:187: // TODO: Call Uniter.OpenPort() On 2013/08/19 15:28:12, fwereade wrote: > and that this should really be talking to a machine, not a unit, and so there's > room for additional smarts here... Perhaps, but I'm trying to replicate the existing usage of state by the uniter. I'll add a TODO. https://codereview.appspot.com/13107043/diff/1/state/api/uniter/unit.go#newco... state/api/uniter/unit.go:194: // TODO: Call Uniter.ClosePort() On 2013/08/19 15:28:12, fwereade wrote: > and here same