resumer: periodically resume transaction

resumer: periodically resume transaction

The resumer is a periodically worker which is
resuming pending transactions in state each
minute.

https://code.launchpad.net/~themue/juju-core/026-transaction-resumer/+merge/169234

(do not edit description out of merge proposal)

[mue, 2013-06-13 15:36:29]

Please take a look.

[fwereade, 2013-06-14 07:33:01]

LGTM

[rog, 2013-06-14 15:46:07]

LGTM with updated doc comment and better tests.

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go
File worker/resumer/resumer.go (right):

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go#newc...
worker/resumer/resumer.go:23: // NewResumer ...
... ?

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go#newc...
worker/resumer/resumer.go:57: log.Errorf("worker/resumer: cannot resume
transactions: %v", err)
to be consistent with the other workers, this error would be fatal.
any particular reason why it's not?

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer_test.go
File worker/resumer/resumer_test.go (right):

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer_test.go...
worker/resumer/resumer_test.go:25: rr := resumer.NewResumer(s.State)
how does this test that ResumeTransactions is actually called? (given that it
actually isn't in this test)

[fwereade, 2013-06-17 13:31:02]

quick responses to rog's comments

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go
File worker/resumer/resumer.go (right):

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go#newc...
worker/resumer/resumer.go:57: log.Errorf("worker/resumer: cannot resume
transactions: %v", err)
On 2013/06/14 15:46:07, rog wrote:
> to be consistent with the other workers, this error would be fatal.
> any particular reason why it's not?

I don't think an error is important enough to justify retrying so much sooner.

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer_test.go
File worker/resumer/resumer_test.go (right):

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer_test.go...
worker/resumer/resumer_test.go:25: rr := resumer.NewResumer(s.State)
On 2013/06/14 15:46:07, rog wrote:
> how does this test that ResumeTransactions is actually called? (given that it
> actually isn't in this test)

Ha. We should ofc use `type Resumer interface { ResumeTransactions() error}` and
check at least that a suitable method gets called on a mock.

[mue, 2013-06-19 11:53:20]

Please take a look.

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go
File worker/resumer/resumer.go (right):

https://codereview.appspot.com/10266043/diff/1/worker/resumer/resumer.go#newc...
worker/resumer/resumer.go:23: // NewResumer ...
On 2013/06/14 15:46:07, rog wrote:
> ... ?

Done.

[fwereade, 2013-06-19 14:28:30]

LGTM