charm: Add config.Convert()

charm: Add config.Convert()

Add a new Config() method to charm config, taking a
map[string]interface{} and returning map[string]interface{},
passing the given values through a schema type checking,
and ignoring the defaults.
This is needed in a follow-up CL that implements most of
upgrade-charm feature's state operations.

https://code.launchpad.net/~dimitern/juju-core/004-upgrade-charm-add-config-convert/+merge/150628

(do not edit description out of merge proposal)

[dimitern, 2013-02-26 17:56:29]

Please take a look.

[rog, 2013-02-26 18:15:10]

LGTM with a couple of minor thoughts.

https://codereview.appspot.com/7405049/diff/1/charm/config.go
File charm/config.go (right):

https://codereview.appspot.com/7405049/diff/1/charm/config.go#newcode122
charm/config.go:122: func (c *Config) Convert(values map[string]interface{})
(converted map[string]interface{}, err error) {
no need to name the return params, i think.

https://codereview.appspot.com/7405049/diff/1/charm/config.go#newcode130
charm/config.go:130: return nil, fmt.Errorf("cannot convert: type of %q has
changed", k)
It might be nice to include a bit more information here to help the user.

return nil, fmt.Errorf("unexpected type in service configuration %q=%#v;
expected %s", k, v, opt.Type)

perhaps?

[dimitern, 2013-02-26 18:24:56]

Please take a look.

https://codereview.appspot.com/7405049/diff/1/charm/config.go
File charm/config.go (right):

https://codereview.appspot.com/7405049/diff/1/charm/config.go#newcode122
charm/config.go:122: func (c *Config) Convert(values map[string]interface{})
(converted map[string]interface{}, err error) {
On 2013/02/26 18:15:10, rog wrote:
> no need to name the return params, i think.

Done.

https://codereview.appspot.com/7405049/diff/1/charm/config.go#newcode130
charm/config.go:130: return nil, fmt.Errorf("cannot convert: type of %q has
changed", k)
On 2013/02/26 18:15:10, rog wrote:
> It might be nice to include a bit more information here to help the user.
> 
> return nil, fmt.Errorf("unexpected type in service configuration %q=%#v;
> expected %s", k, v, opt.Type)
> 
> perhaps?

Done.

[dave_cheney.net, 2013-02-27 05:49:20]

https://codereview.appspot.com/7405049/diff/5001/charm/config_test.go
File charm/config_test.go (right):

https://codereview.appspot.com/7405049/diff/5001/charm/config_test.go#newcode193
charm/config_test.go:193: func (s *ConfigSuite) TestConvert(c *C) {
Could you convert this into a table driven test ? At least the input == expected
parts ?

[dimitern, 2013-02-27 08:19:16]

Please take a look.

https://codereview.appspot.com/7405049/diff/5001/charm/config_test.go
File charm/config_test.go (right):

https://codereview.appspot.com/7405049/diff/5001/charm/config_test.go#newcode193
charm/config_test.go:193: func (s *ConfigSuite) TestConvert(c *C) {
On 2013/02/27 05:49:20, dfc wrote:
> Could you convert this into a table driven test ? At least the input ==
expected
> parts ?

Good point! Done.

[dave_cheney.net, 2013-02-27 08:37:25]

LGTM. Nice tests.

[dimitern, 2013-02-27 08:42:13]

*** Submitted:

charm: Add config.Convert()

Add a new Config() method to charm config, taking a
map[string]interface{} and returning map[string]interface{},
passing the given values through a schema type checking,
and ignoring the defaults.
This is needed in a follow-up CL that implements most of
upgrade-charm feature's state operations.

R=rog, dfc
CC=
https://codereview.appspot.com/7405049