rework state charm usage

rework state charm usage

* Nothing has CharmURL or SetCharmURL methods any more
* Unit lost all upgrade-related methods
* Service.SetCharm also takes a force bool param
* Service.Charm returns a Charm with an additional Force field
* Service.WatchCharm produces the same thing that Service.Charm does
* oh, and, removed uniter/charm.UpgradingForced

https://code.launchpad.net/~fwereade/juju-core/rework-state-charms/+merge/120566

(do not edit description out of merge proposal)

[fwereade, 2012-08-21 13:36:44]

Please take a look.

[niemeyer, 2012-08-21 13:45:50]

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

https://codereview.appspot.com/6441169/diff/1/state/service.go#newcode31
state/service.go:31: // ServiceCharm describes the charm that units of the
service should use.
It feels straightforward and handy to have two parameters to the methods rather
than another type here. The documentation of this type and of the Charm method
match pretty much exactly.

https://codereview.appspot.com/6441169/diff/1/state/service.go#newcode73
state/service.go:73: setCharm := func(oldYaml string, _ *zookeeper.Stat)
(string, error) {
Can't we just use a config node with the two parameters, and then use these two
parameters in the two functions?

[fwereade, 2012-08-21 14:54:39]

Please take a look.

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

https://codereview.appspot.com/6441169/diff/1/state/service.go#newcode31
state/service.go:31: // ServiceCharm describes the charm that units of the
service should use.
On 2012/08/21 13:45:50, niemeyer wrote:
> It feels straightforward and handy to have two parameters to the methods
rather
> than another type here. The documentation of this type and of the Charm method
> match pretty much exactly. 

Done.

https://codereview.appspot.com/6441169/diff/1/state/service.go#newcode73
state/service.go:73: setCharm := func(oldYaml string, _ *zookeeper.Stat)
(string, error) {
On 2012/08/21 13:45:50, niemeyer wrote:
> Can't we just use a config node with the two parameters, and then use these
two
> parameters in the two functions?

Done.

[niemeyer, 2012-08-21 15:07:36]

Thanks, LGTM given the following details sorted:

https://codereview.appspot.com/6441169/diff/4002/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/6441169/diff/4002/state/service_test.go#newcode33
state/service_test.go:33: // TODO: changing the charm like this is not
especially sane in itself.
// TODO SetCharm must validate the change (version, relations, etc).

https://codereview.appspot.com/6441169/diff/4002/state/service_test.go#newcod...
state/service_test.go:275: w := s.service.WatchCharm()
defer w.Stop()?

https://codereview.appspot.com/6441169/diff/4002/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6441169/diff/4002/state/state.go#newcode171
state/state.go:171: "charm-url":   ch.URL().String(),
A charm URL stringifies itself correctly when being inserted.

It also feels like this should be "charm" rather than "charm-url" (parallel with
"unit-name" vs. "unit", "service-name" vs. "service", "machine-id" vs.
"machine").

[fwereade, 2012-08-21 15:41:45]

*** Submitted:

rework state charm usage

* Nothing has CharmURL or SetCharmURL methods any more
* Unit lost all upgrade-related methods
* Service.SetCharm also takes a force bool param
* Service.Charm returns a Charm with an additional Force field
* Service.WatchCharm produces the same thing that Service.Charm does
* oh, and, removed uniter/charm.UpgradingForced

R=niemeyer
CC=
https://codereview.appspot.com/6441169

https://codereview.appspot.com/6441169/diff/4002/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/6441169/diff/4002/state/service_test.go#newcode33
state/service_test.go:33: // TODO: changing the charm like this is not
especially sane in itself.
On 2012/08/21 15:07:36, niemeyer wrote:
> // TODO SetCharm must validate the change (version, relations, etc).

Done.

https://codereview.appspot.com/6441169/diff/4002/state/service_test.go#newcod...
state/service_test.go:275: w := s.service.WatchCharm()
On 2012/08/21 15:07:36, niemeyer wrote:
> defer w.Stop()?

Done.

https://codereview.appspot.com/6441169/diff/4002/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6441169/diff/4002/state/state.go#newcode171
state/state.go:171: "charm-url":   ch.URL().String(),
On 2012/08/21 15:07:36, niemeyer wrote:
> A charm URL stringifies itself correctly when being inserted.
> 
> It also feels like this should be "charm" rather than "charm-url" (parallel
with
> "unit-name" vs. "unit", "service-name" vs. "service", "machine-id" vs.
> "machine").

Done.