Code review comment for lp:~dimitern/juju-core/148-apiuniter-fix-configsettings

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/13908044/diff/1/state/api/uniter/unit.go
File state/api/uniter/unit.go (right):

https://codereview.appspot.com/13908044/diff/1/state/api/uniter/unit.go#newcode143
state/api/uniter/unit.go:143: return
map[string]interface{}(result.Settings), nil
On 2013/09/26 16:11:52, fwereade wrote:
> can you not do charm.Settings(result.Settings)?

Done.

https://codereview.appspot.com/13908044/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (left):

https://codereview.appspot.com/13908044/diff/1/state/apiserver/uniter/uniter.go#oldcode797
state/apiserver/uniter/uniter.go:797: // All relation settings should be
strings.
On 2013/09/26 16:11:52, fwereade wrote:
> Hmm. This is almost legitimate-panic territory. Best not, since we're
running in
> the API server and will cause a lot of collateral damage, but I think
we should
> at least error out -- if it happens, we're clearly not in Kansas any
more.

Done. Also added a couple of tests for the error.

https://codereview.appspot.com/13908044/

« Back to merge proposal