search

python-zk / kazoo

Kazoo is a high-level Python library that makes it easier to use Apache Zookeeper.
https://kazoo.readthedocs.io
Apache License 2.0
1.3k stars 386 forks source link

feat: add support for persistent recursive watches #715

Open jeblair opened 1 year ago

jeblair commented 1 year ago

Why is this needed?

ZooKeeper 3.6.0 added support for persistent, and persistent recursive watches. This adds the corresponding support to the Kazoo client class.

Does this PR introduce any breaking change?

No breaking changes.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.56%. Comparing base (2fb93a8) to head (415dc93). Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #715 +/- ## ========================================== - Coverage 96.81% 96.56% -0.26% ========================================== Files 27 27 Lines 3549 3639 +90 ========================================== + Hits 3436 3514 +78 - Misses 113 125 +12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jeblair commented 1 year ago

It looks like the test failures are not related to this change.

kt315 commented 8 months ago

Hi! Why not merge this?

kt315 commented 7 months ago

Hi! Whose approval is need? 🤔

StephenSorriaux commented 7 months ago

Hello,

Approval from @python-zk/maintainers is needed. Will try to take a look by end of the week.

Can somebody that already tried the feature can confirm it is working as expected?

jeblair commented 7 months ago

Hello,

Approval from @python-zk/maintainers is needed. Will try to take a look by end of the week.

Can somebody that already tried the feature can confirm it is working as expected?

While awaiting review/merge we in the Zuul project have vendored a copy[1] of this and used it successfully in production for about nine months (though it does look like our vendored copy omits the ability to remove watches since we never do so, so we'll have to rely on the tests for that). Thanks for taking a look. :)

[1] https://opendev.org/zuul/nodepool/src/branch/master/nodepool/zk/vendor

StephenSorriaux commented 5 months ago

@jeffwidman @ceache @a-ungurianu this one is now ready to be reviewed @jeblair I made some small changes, mostly based on my previous review, please let me now if you see anything wrong

jeblair commented 5 months ago

@jeffwidman @ceache @a-ungurianu this one is now ready to be reviewed @jeblair I made some small changes, mostly based on my previous review, please let me now if you see anything wrong

The changes LGTM, thanks! And sorry I wasn't able to address those myself in a timely manner.

StephenSorriaux commented 5 months ago

Thank you @jeblair

@jeffwidman @ceache @a-ungurianu gentle bump!