-
Notifications
You must be signed in to change notification settings - Fork 7.3k
ZOOKEEPER-3281:Add a new CLI:watch #822
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
Conversation
d7b37c8 to
10e8bc5
Compare
| if (e.getType() == Event.EventType.None) { | ||
| return; | ||
| } | ||
| if (e.getState() == Event.KeeperState.SyncConnected) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this capture every state that might be encountered? Would it be possible to receive a Disconnected or an AuthFailed event and is there something that might gracefully handle them?
The code for this change looks good to me outside of these questions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@enixon Yes,agreed!
ad47991 to
3466647
Compare
3466647 to
5b06ba3
Compare
|
I like this. Code looks good to me - my only thought for improvement is whether we can move the action on the watch firing into the callback so that this operation can be nonblocking. Anyone else have thoughts? |
49ba876 to
af9bff4
Compare
|
anmolnar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 lgtm.
Please rebase.
watchfunction.watchcmd is one-off and it will be blocking until something has changed.ls -w /footo watch the child change,get -w /footo watch the data change,this new cli is also valuable and user-fridendly in a perspective view of watch.-foption: watch a node forever, add-Roption to watch all the events under a dir.watchin thezookeeper.java,the correctness of function has been tested in theWatcherFuncTest,so add no UTs.But a very detailed test evidence has included in the jira