Merge lp:~free.ekanayaka/storm/track-retries into lp:storm

[Free Ekanayaka]

This branch adds a new on_retry instance variable to the storm.twisted.Transactor, which can be set to a function that will be called back just before executing a retry.

[lp:~free.ekanayaka/storm/track-retries]

427. By Free Ekanayaka on 2011-11-30

Call transaction.begin() at each try iteration

428. By Free Ekanayaka on 2011-12-01

Reverted the transaction.begin() change for now, because it causes uncommitted test data to be lost

[Thomas Herve (therve)]

+1!

review: Approve

[Björn Tillenius (bjornt)]

[1]

+class RetryContext(object):
+ """Hold details about a function that is going to be retried.
+
+ @ivar function: The function that is going to be retried.
+ @ivar args: The positional arguments passed to the function.
+ @ivar kwargs: The keyword arguments passed to the function.
+ @ivar retry: The sequential number of the retry that is going to be
+ performed.
+ """

Wouldn't it make sense to include the error that caused the retry to happen?

[2]

+ sleep = time.sleep
+ uniform = random.uniform

Using methods on Transactor makes sense, in order to reduce the need of mocking. However, I see that in the tests you still mock these methods. Unless the tests are explicitly testing, asserting that those methods are called the right amount of times just adds noise to the test. So I'd say that unless you mention the sleep behavior in the test docstring, don't mock them. Better to have a separate test that tests the sleep behavior.

review: Approve

[lp:~free.ekanayaka/storm/track-retries]

429. By Free Ekanayaka on 2011-12-07

Include the error in RetryContext

[Free Ekanayaka (free.ekanayaka)]

Thanks!

[1] Fixed

[2] As discussed, these tests were already there and I didn't want to change them. Also, they do check that sleep() and random() are called in a certain order and with certain arguments.