charm: allow empty strings as config values

charm: allow empty strings as config values

This is the second CL for lp:1194945. It allows empty strings
as valid values for config options with the type string. Merging
should be done after none has signalled a compatibility problem
as answer to the query mail on the mailing list.

https://code.launchpad.net/~themue/juju-core/037-empty-strings-in-charm-config/+merge/178318

Requires: https://code.launchpad.net/~themue/juju-core/036-set-default/+merge/178279

(do not edit description out of merge proposal)

[mue, 2013-08-02 15:02:57]

Please take a look.

[dimitern, 2013-08-02 15:59:33]

LGTM

[jameinel, 2013-08-08 16:57:55]

LGTM

[fwereade, 2013-08-09 13:19:05]

Sorry, NOT LGTM without explanation. The API has lost/changed functionality, I
think, because we can no longer set string values to defaults; and we've now got
two ways to set defaults for non-string types. How is all this going to work in
the imminent CLI-API world?

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

https://codereview.appspot.com/12343045/diff/1/charm/config.go#newcode39
charm/config.go:39: // are always considered valid, and empty string values are
converted to nil.
This isn't true any more.

https://codereview.appspot.com/12343045/diff/1/charm/config.go#newcode48
charm/config.go:48: } else if option.Type != "string" && value == "" {
Why are we keeping this behaviour? Having `value=` mean different things
depending on type seems like a bad idea to me. Can't we just delete this whole
special-case block?

https://codereview.appspot.com/12343045/diff/1/charm/config.go#newcode67
charm/config.go:67: if option.Type != "string" && str == "" {
Similarly.

https://codereview.appspot.com/12343045/diff/1/charm/config_test.go
File charm/config_test.go (right):

https://codereview.appspot.com/12343045/diff/1/charm/config_test.go#newcode337
charm/config_test.go:337: info: "empty strings are nil values for non-string
types",
Yeah -- why do we now have two ways of specifying set-to-default? I don't think
this is a good idea *unless* we have to support `value=` for python
compatibility.

https://codereview.appspot.com/12343045/diff/1/state/statecmd/config_test.go
File state/statecmd/config_test.go (right):

https://codereview.appspot.com/12343045/diff/1/state/statecmd/config_test.go#...
state/statecmd/config_test.go:90: "value":       "",
So how do we set defaults over the API?