Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Add support for Container and TTL nodes #613

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ceache
Copy link
Contributor

@ceache ceache commented Jun 16, 2020

Also add support through transations.

Closes #334, #496

kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
@ceache ceache force-pushed the feat/container_ttl branch 2 times, most recently from 571168e to 696e51e Compare June 16, 2020 04:54
@ceache
Copy link
Contributor Author

ceache commented Jun 16, 2020

Still missing tests. Will need to update the harness to configure Zookeeper to enable extended types.

@jeffwidman
Copy link
Member

@ceache just curious on the status here? I assume you've just been busy? Or have you abandoned this for some reason and forgot to close it?

@ceache
Copy link
Contributor Author

ceache commented Dec 13, 2020 via email

@jeffwidman
Copy link
Member

Haha, I also have been working on a Wireshark zookeeper dissector the last few months in my spare time as a way to learn C/wiresharp APIs/understand the ZK protocol at a deeper level: https://gitlab.com/wireshark/wireshark/-/merge_requests/528

I've had to put it on hold the last few weeks as we somewhat spontaneously decided to buy a house so I've been busy with that, but hope to pick it back up during my time off over the holidays.

I would say for testing, being permissive is fine with me. I'm also completely fine with letting this sit for a bit longer to get a ZK release with the bugfix if that makes testing this simpler... whatever you prefer/think is wisest. It would be nice to have a test, but I defer to your judgement. I just wanted a status update since there'd been no update since June.

@ceache
Copy link
Contributor Author

ceache commented Dec 14, 2020

Mine is a LUA dissector https://github.com/ceache/zab_dissector

@Traktormaster
Copy link

Hi!

In short I've found myself in the need for nodes with TTL. I'm not entirely sure how this PR is blocked, but I figured it might be good enough for me. I've forked, merged it in and added a simple test to check if persistent TTL nodes behave as expected. They work great as far as I can tell. You can find it here: https://github.com/Traktormaster/kazoo/tree/ttlme

I'm not sure how to offer that commit into this PR myself, but you may find it useful when getting back to work on this.

Thanks!

@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2022

Codecov Report

Attention: Patch coverage is 60.60606% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 93.58%. Comparing base (2fb93a8) to head (69a3271).
Report is 2 commits behind head on master.

❗ Current head 69a3271 differs from pull request most recent head db886ae. Consider uploading reports for the commit db886ae to get more accurate results

Files Patch % Lines
kazoo/protocol/serialization.py 40.42% 28 Missing ⚠️
kazoo/client.py 78.84% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #613      +/-   ##
==========================================
- Coverage   96.81%   93.58%   -3.24%     
==========================================
  Files          27       57      +30     
  Lines        3549     8418    +4869     
==========================================
+ Hits         3436     7878    +4442     
- Misses        113      540     +427     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ceache ceache force-pushed the feat/container_ttl branch 2 times, most recently from 26a151c to b9c6d51 Compare June 2, 2022 17:36
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
kazoo/protocol/serialization.py Outdated Show resolved Hide resolved
@ceache ceache force-pushed the feat/container_ttl branch 2 times, most recently from fb9c9dc to 84a0024 Compare June 2, 2022 17:49
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
@j-smith-1574
Copy link

Hello,

is there any updates on the release of this PR?

kazoo/client.py Outdated Show resolved Hide resolved
kazoo/client.py Outdated Show resolved Hide resolved
@ceache ceache force-pushed the feat/container_ttl branch 2 times, most recently from 476d616 to b030864 Compare February 12, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support container znodes
6 participants