Code review comment for lp://staging/~hazmat/txzookeeper/backoff-retry-managed-sanity

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

the test for backoff are in test_session which operates a full zk cluster
and has a test case that verifies backoff has run. there's a new makefile
with one target .. make coverage that runs through the full test suite. in
testing with juju environments, i discovered another minor issue with
retry.

thanks for the review.

-k

On Wed, Feb 6, 2013 at 7:15 PM, Benjamin Saller <
<email address hidden>> wrote:

> Hi,
>
> Thanks for the branch. Initially I had an older version of pyzookeeper in
> my path and the tests were segfaulting, but after tracking that down I was
> able to get things working smoothly.
>
> Coverage shows that the session backoff code isn't tested, if I'm reading
> it properly self._backoff_seconds is never incremented in the tests and so
> some of the focus of this branch goes without regression testing.
>
> It looks like retry_error_callback never triggers cb_retry_expired in the
> tests either.
>
> Nice simplification in node.py.
>
> This looks good but I would like to see a few more tests around the retry
> path.
>
>
>
> --
>
> https://code.launchpad.net/~hazmat/txzookeeper/backoff-retry-managed-sanity/+merge/146938
> You are the owner of lp:~hazmat/txzookeeper/backoff-retry-managed-sanity.
>

« Back to merge proposal