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#newcode57 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#newcode25 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.
https://codereview.appspot.com/10266043/
« Back to merge proposal
quick responses to rog's comments
https:/ /codereview. appspot. com/10266043/ diff/1/ worker/ resumer/ resumer. go resumer/ resumer. go (right):
File worker/
https:/ /codereview. appspot. com/10266043/ diff/1/ worker/ resumer/ resumer. go#newcode57 resumer/ resumer. go:57: log.Errorf( "worker/ resumer: cannot resume
worker/
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 resumer/ resumer_ test.go (right):
File worker/
https:/ /codereview. appspot. com/10266043/ diff/1/ worker/ resumer/ resumer_ test.go# newcode25 resumer/ resumer_ test.go: 25: rr := resumer. NewResumer( s.State)
worker/
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 { ResumeTransacti ons()
error}` and check at least that a suitable method gets called on a mock.
https:/ /codereview. appspot. com/10266043/