New worker to run the charm version updater

New worker to run the charm version updater

A new worker is added to state servers, which
runs the charm version updater every 6 hours.

https://code.launchpad.net/~wallyworld/juju-core/charm-version-worker/+merge/200948

Requires: https://code.launchpad.net/~wallyworld/juju-core/charm-version-updater/+merge/200758

(do not edit description out of merge proposal)

[wallyworld, 2014-01-09 03:35:25]

Please take a look.

[wallyworld, 2014-01-15 08:03:35]

Please take a look.

[wallyworld, 2014-01-15 23:26:56]

Please take a look.

[thumper, 2014-01-17 02:21:06]

https://codereview.appspot.com/49500043/diff/40001/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/49500043/diff/40001/cmd/jujud/machine.go#newco...
cmd/jujud/machine.go:212: runner.StartWorker("charm-revision-updater", func()
(worker.Worker, error) {
Oh yeah. That's how we roll. Implement it right first time.

https://codereview.appspot.com/49500043/diff/40001/cmd/jujud/machine.go#newco...
cmd/jujud/machine.go:273: // a *state.State connection.
nice.

https://codereview.appspot.com/49500043/diff/40001/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/49500043/diff/40001/cmd/jujud/machine_test.go#...
cmd/jujud/machine_test.go:724: c.Assert(err, gc.IsNil)
OMG why hasn't anyone written a helper for this... not expecting you to do it,
but more just a personal exclamation.

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
File worker/charmrevisionworker/revisionupdater.go (right):

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater.go:18: const defaultInterval = 6 *
time.Hour
6 hours? I thought the initial thought was daily?

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater.go:21: var interval = defaultInterval
Why do we need this? I don't understand why you need interval AN
defaultInterval.

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater.go:76: log.Errorf("worker/charm
revision lookup: cannot process charms: %v", err)
Please replace log.Errorf with logger.Errorf and fix the logging message.

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
File worker/charmrevisionworker/revisionupdater_test.go (right):

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater_test.go:36:
c.Assert(*charmrevisionworker.Interval, gc.Equals, 6*time.Hour)
Really? Why?

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater_test.go:56:
c.Assert(s.versionUpdater.Stop(), gc.IsNil)
Why not do this with addCleanup?

[wallyworld, 2014-01-17 13:26:07]

Please take a look.

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
File worker/charmrevisionworker/revisionupdater.go (right):

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater.go:18: const defaultInterval = 6 *
time.Hour
On 2014/01/17 02:21:07, thumper wrote:
> 6 hours? I thought the initial thought was daily?

It was but I thought 1 day was too long to wait if a charm has been updated.

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater.go:21: var interval = defaultInterval
On 2014/01/17 02:21:07, thumper wrote:
> Why do we need this? I don't understand why you need interval AN
> defaultInterval.

One is a const, the other is a var (initialised to the const) that is patched
for testing. I guess I can lose the const

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
File worker/charmrevisionworker/revisionupdater_test.go (right):

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater_test.go:36:
c.Assert(*charmrevisionworker.Interval, gc.Equals, 6*time.Hour)
On 2014/01/17 02:21:07, thumper wrote:
> Really? Why?

Cause I thought 24 hours was too long to wait if a charm has been updated

https://codereview.appspot.com/49500043/diff/40001/worker/charmrevisionworker...
worker/charmrevisionworker/revisionupdater_test.go:56:
c.Assert(s.versionUpdater.Stop(), gc.IsNil)
On 2014/01/17 02:21:07, thumper wrote:
> Why not do this with addCleanup?

Can do

[wallyworld, 2014-01-17 13:36:54]

Please take a look.

[fwereade, 2014-01-17 13:39:20]

LGTM with the 24h fix, thanks very much.