Merge lp://staging/~fwereade/juju-core/config-5-state-service-config-yaml into lp://staging/~juju/juju-core/trunk

Reviewers: mp+168579_code.launchpad.net,

Message:
Please take a look.

Description:
state: Service.SetConfigYAML

...now works as intended. Don't get too attached to it though.

https://code.launchpad.net/~fwereade/juju-core/config-5-state-service-config-yaml/+merge/168579

Requires:
https://code.launchpad.net/~fwereade/juju-core/config-4-state-unit-settings-watcher/+merge/168578

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/juju/config_test.go
   M juju/conn_test.go
   M state/apiserver/client_test.go
   M state/apiserver/perm_test.go
   M state/service.go
   M state/service_test.go
   M state/statecmd/deploy_test.go

LGTM - comment about a bug maybe fixed

https://codereview.appspot.com/10172045/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/10172045/diff/1/state/service.go#newcode778
state/service.go:778: charm, _, err := s.Charm()
Is lp:1167465 being addressed by this?

https://codereview.appspot.com/10172045/

LGTM

https://codereview.appspot.com/10172045/

LGTM, thanks.

https://codereview.appspot.com/10172045/diff/1/cmd/juju/config_test.go
File cmd/juju/config_test.go (right):

https://codereview.appspot.com/10172045/diff/1/cmd/juju/config_test.go#newcode159
cmd/juju/config_test.go:159: path := ctx.AbsPath("testconfig.yaml")
err := ioutil.WriteFile(
     ctx.AbsPath("testconfig.yaml"),
     []byte("dummy-service:\n skill-level: 9000\n username: admin
      001\n\n")),
     0666)
c.Assert(err, IsNil)

?

https://codereview.appspot.com/10172045/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/10172045/diff/1/state/service.go#newcode748
state/service.go:748: _, err = node.Write()
does anything ever use the return from Write?

https://codereview.appspot.com/10172045/

*** Submitted:

state: Service.SetConfigYAML

...now works as intended. Don't get too attached to it though.

R=thumper, wallyworld, rog
CC=
https://codereview.appspot.com/10172045

https://codereview.appspot.com/10172045/diff/1/cmd/juju/config_test.go
File cmd/juju/config_test.go (right):

https://codereview.appspot.com/10172045/diff/1/cmd/juju/config_test.go#newcode159
cmd/juju/config_test.go:159: path := ctx.AbsPath("testconfig.yaml")
On 2013/06/11 12:21:01, rog wrote:
> err := ioutil.WriteFile(
> ctx.AbsPath("testconfig.yaml"),
> []byte("dummy-service:\n skill-level: 9000\n username: admin
> 001\n\n")),
> 0666)
> c.Assert(err, IsNil)

> ?

Why not indeed. Done.

https://codereview.appspot.com/10172045/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/10172045/diff/1/state/service.go#newcode748
state/service.go:748: _, err = node.Write()
On 2013/06/11 12:21:01, rog wrote:
> does anything ever use the return from Write?

state.Settings is madness through and through. I suspect that the only
reason for that return value is to effectively expose the algorithm for
testing purposes.

https://codereview.appspot.com/10172045/diff/1/state/service.go#newcode778
state/service.go:778: charm, _, err := s.Charm()
On 2013/06/11 05:22:42, thumper wrote:
> Is lp:1167465 being addressed by this?

Yeah :). Done.

https://codereview.appspot.com/10172045/