Code review comment for lp://staging/~free.ekanayaka/storm/track-retries

[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.