feat(recipe): add ExistingDataWatch class

[jeblair]

This adds a subclass of DataWatch which only operates on existing ZNodes. If a user uses a DataWatch on a path and the ZNode at that path is deleted, the DataWatch will still issue an "exists" call and set a watch right before the final callback. That means that regardless of the return value of the callback and whether or not Kazoo will invoke the callback again, the ZooKeeper server still has a watch entry for that path.

In short, using a DataWatch on a path and then deleting that path can leak watch entries on the ZooKeeper server.

Because the DataWatch recipe is designed to watch non-existing paths, this behavior may be desired and relied on by some users, so it's not considered a bug. But other users may want to use DataWatches for nodes where this behavior would be a problem.

The ExistingDataWatch class behaves similarly to its parent class, DataWatch, but it does not set a watch on paths which do not exist (whether that's because they never existed or were recently deleted). This means that a user of an ExistingDataWatch can be assured that after the callback with the deleted event, the watch is removed from the server.

[ceache]

Resurrecting this PR conversation a little bit.

I am just wondering in which case one would use DataWatch over ExistingDataWatch? The leak issue rational is clear in the comment above, but what do you lose by removing that watch?

In short, I am wondering why we would want a different class instead of putting the fix in Datawatch proper.

[codecov-commenter]

Codecov Report

Merging #648 (48f5412) into master (9bb8499) will decrease coverage by 0.43%. The diff coverage is 98.70%.

@@            Coverage Diff             @@
##           master     #648      +/-   ##
==========================================
- Coverage   94.47%   94.03%   -0.44%     
==========================================
  Files          57       57              
  Lines        8375     8451      +76     
==========================================
+ Hits         7912     7947      +35     
- Misses        463      504      +41     

Impacted Files	Coverage Δ
kazoo/recipe/watchers.py	93.75% <95.00%> (+0.14%)	:arrow_up:
kazoo/client.py	97.93% <100.00%> (-0.48%)	:arrow_down:
kazoo/tests/test_watchers.py	99.14% <100.00%> (+0.11%)	:arrow_up:
kazoo/tests/test_cache.py	54.42% <0.00%> (-3.75%)	:arrow_down:
kazoo/protocol/connection.py	92.37% <0.00%> (-2.69%)	:arrow_down:
kazoo/testing/harness.py	95.83% <0.00%> (-2.50%)	:arrow_down:
kazoo/handlers/gevent.py	92.47% <0.00%> (-2.16%)	:arrow_down:
kazoo/tests/test_eventlet_handler.py	87.97% <0.00%> (-1.64%)	:arrow_down:
kazoo/tests/util.py	62.50% <0.00%> (-1.39%)	:arrow_down:
kazoo/handlers/eventlet.py	99.01% <0.00%> (-0.99%)	:arrow_down:
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9bb8499...48f5412. Read the comment docs.

[jeblair]

A possible use case for DataWatch is to wait for a znode to appear when none is present already. I believe I wasn't able to see a way to eliminate the leak when a znode is removed while also supporting that behavior. I'm fuzzy on the details (it's been a while!) but I think that's the key assumption I was working from.