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

add etcd watch design documentation #40

Merged
merged 1 commit into from
Aug 25, 2016
Merged

Conversation

StupidHod
Copy link
Contributor

Hope for your advice, @xiang90 @adohe

@adohe-zz
Copy link

👍 this should be a good beginning of watch.

@@ -0,0 +1,88 @@
# Overview

The EtcdWatch provide methods to watch on a key and cancel a watcher, if the watcher is disconnected on error, it will be resumed automatically.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"cancel a watcher. If ..."
Use "."

@hongchaodeng
Copy link
Contributor

@StupidHod @adohe
Maybe we can start with the goals to have EtcdWatcher, and then talk about details about how to implement each?

@adohe-zz
Copy link

@hongchaodeng yes, we should examine the goals before we start the implementation.

@StupidHod StupidHod force-pushed the watch branch 5 times, most recently from 91f6699 to f6e9dea Compare August 22, 2016 07:06
@StupidHod StupidHod mentioned this pull request Aug 24, 2016
@StupidHod StupidHod force-pushed the watch branch 3 times, most recently from 23c28d6 to 3e8d91e Compare August 24, 2016 08:15
@xiang90
Copy link
Contributor

xiang90 commented Aug 24, 2016

@StupidHod So this works pretty similar to the go version?

@@ -0,0 +1,110 @@
# Overview

The EtcdWatch provide methods to watch on a key and cancel a watcher. If the watcher is disconnected on error, it will be resumed automatically.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/key/key interval

@xiang90
Copy link
Contributor

xiang90 commented Aug 24, 2016

The design doc looks good. Let's clean it up and get it merged.

@StupidHod
Copy link
Contributor Author

@xiang90 , thanks for your advice, I have cleared it up with your tips.

@@ -0,0 +1,110 @@
# Overview
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace all Etcd with etcd (including file name)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@StupidHod
Copy link
Contributor Author

Ok, I have corrected it.

@hongchaodeng
Copy link
Contributor

LGTM
Let's move forward with implementation.
Thanks @StupidHod !

@hongchaodeng hongchaodeng merged commit 845be1e into etcd-io:master Aug 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants