Code review comment for lp:~hazmat/txzookeeper/managed-watch-and-ephemeral

On 2012/04/04 13:23:29, fwereade wrote:
> This looks very nice indeed, but the vagueness of the "comes at a
cost" comment
> makes me wonder what I'm missing...

expanded greatly what those costs are.

> https://codereview.appspot.com/5976074/diff/1/txzookeeper/managed.py
> File txzookeeper/managed.py (right):

https://codereview.appspot.com/5976074/diff/1/txzookeeper/managed.py#newcode66
> txzookeeper/managed.py:66: mgr # keep the flakes happy
> Can't you just do:

> with self._ctx():

> ?

much nicer, thanks.

https://codereview.appspot.com/5976074/diff/1/txzookeeper/managed.py#newcode70
> txzookeeper/managed.py:70: mgr # keep the flakes happy
> Ditto

done.

https://codereview.appspot.com/5976074/diff/1/txzookeeper/managed.py#newcode122
> txzookeeper/managed.py:122: come at a cost though.
> Please expand ;).

done.

https://codereview.appspot.com/5976074/diff/1/txzookeeper/managed.py#newcode138
> txzookeeper/managed.py:138: """Called on intercept of session
expiration to
> restablish the session.
> I'm pretty sure there are 2 es in re-establish, and that the hyphen
may have
> fallen out of common use but remains IMO more readable. As you wish.

proper speling established ;-)

https://codereview.appspot.com/5976074/diff/1/txzookeeper/managed.py#newcode140
> txzookeeper/managed.py:140: This will reconnect to zk, restabslish
ephemerals,
> and trigger watches.
> I like the word "restabslish", but ditto.

https://codereview.appspot.com/5976074/diff/1/txzookeeper/tests/test_managed.py
> File txzookeeper/tests/test_managed.py (right):

https://codereview.appspot.com/5976074/diff/1/txzookeeper/tests/test_managed.py#newcode169
> txzookeeper/tests/test_managed.py:169: # It takes some time to
propogate (1/3
> session time as ping)
> s/propogate/propagate/

done.

http://codereview.appspot.com/5976074/