allow_session_lost ignored for DataWatch

[jimfulton]

I have 2 connections against 2 tunnels

>>> import kazoo.client
>>> c = kazoo.client.KazooClient('localhost:2182')
>>> c2 = kazoo.client.KazooClient('localhost:2183')
>>> @c.add_listener
... def _(state):
...     print state
... 
>>> c.start()
CONNECTED
>>> c2.start()
>>> @c.DataWatch('/test', allow_session_lost=True)
... def _(data, *a):
...     print True, data
... 
True q
>>> @c.DataWatch('/test', allow_session_lost=False)
... def _(data, *a):
...     print False, data
... 
False q

I shut down the first tunnel.

SUSPENDED

I wait a while and restart it:

LOST
CONNECTED

>>> _ = c2.set('test', 'a')

False a
True a

Both of the watches are re-established. Only one should be, although I don't know which. :)

[bbangert]

allow_session_lost is completely ignored in newer Kazoo. This was one of the simplifications made because DataWatch was getting crufty. (See other issue that I will finish replying to in a few seconds).

If you want to stop watching, your function should take more arguments so that it can see the session loss event and return False thus deregistering.

[jimfulton]

I'm happy to have data and children watchers stay forever. I was just trying to follow the documentation to make surer this happens.

[bbangert]

Yea, making the children watches last forever will be tougher since it'll have to throw a exist watch if the node is deleted to be notified of when/if it comes back. What doc string needs to be updated? Maybe refile this as a documentation bug?

[jimfulton]

I'm OK with anything as long as I know what it is. :)

As it is, setting allow_session_lost to True makes child watches span multiple sessions.
So ignoring allow_session_lost for ChildrenWatch as well as DataWatch seems straightforward.

I'm happy to make a documentation PR when you decide on the semantics.

Node deletion is a different issue. (literally).