etcdctl: support exec watch in v3 #8919
Conversation
gyuho
force-pushed
the
exec-watch
branch
2 times, most recently
from
fc8db4f
to
ca7384b
Compare
gyuho
force-pushed
the
exec-watch
branch
2 times, most recently
from
7ffd56f
to
db00ff0
Compare
|
/cc @jpbetz can you take a look? |
|
Sure, I’ll have a look |
|
I'd love to see this released. This blocks migration of APIv2 to APIv3. Are there any short-term hacks for 3.2.10 or 3.2.11 to run a command on a change of watched value? |
|
@vikin91 We plan to release this in 3.3 (release candidate will be out this or early next year). |
|
Sure, I will try it soon and report my experience. |
|
@gyuho , I tried the code and it works good. However, there is one difference between APIv2 and APIv3. In APIv2, the |
|
@vikin91 Thanks for trying out! I would like to keep it as simple as possible for now--environment variable support for watch key/value can be easily added later. ref. |
| return nil, fmt.Errorf("bad number of arguments") | ||
| func getWatchChan(c *clientv3.Client, args []string, interactive bool) (clientv3.WatchChan, []string, error) { | ||
| if interactive { | ||
| args = args[1:] // args[0]=="watch" |
There was a problem hiding this comment.
Move the assertion in this comment into the code, e.g.: if args[0] != "watch { panic("args[0] must be 'watch' for interactive calls") } ?
| } | ||
| } | ||
| osArgs := os.Args[idx+1:] | ||
| execArgs, err = findExecArgs(osArgs) |
There was a problem hiding this comment.
Avoid repeating the findExecArgs call by restructuring this as:
if !interactive {
...
args := osArgs
}
execArgs, err = findExecArgs(args)
?
Also, should findExecArgs return both the args before -- as well as the args after -- so that the subsequent code can deal with them as cleanly separated lists? I'm concerned that checks like len(args) == 2 below might not make sense if we don't cleanly separate the two args lists. I add another comment about this below for findExecArgs.
| return nil, nil | ||
| } | ||
|
|
||
| if idx == len(args)-1 { |
There was a problem hiding this comment.
Let's split the args and execArgs into two distinct lists here and then simplify all following, logic, e.g.:
func findExecArgs(args []string) ([]string, []string, error) {
foundSep, idx := false, 0
for idx = range args {
if args[idx] == "--" && idx != 0 {
foundSep = true
break
}
}
if !foundSep {
return args, nil, nil
}
watchArgs, execArgs := args[:idx], args[idx+1:]
// check for various invalid states
...
return watchArgs, execArgs, nil
}
Signed-off-by: Gyu-Ho Lee <[email protected]>
Signed-off-by: Gyu-Ho Lee <[email protected]>
Signed-off-by: Gyu-Ho Lee <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #8919 +/- ##
==========================================
- Coverage 76.08% 75.91% -0.18%
==========================================
Files 359 359
Lines 29841 29891 +50
==========================================
- Hits 22704 22691 -13
- Misses 5559 5619 +60
- Partials 1578 1581 +3
Continue to review full report at Codecov.
|
|
@jpbetz PTAL. Refactored Thanks! |
|
As reported by @vikin91, the feature is not usable without the environment variables (ETCD_WATCH_KEY, ETCD_WATCH_VALUE...) being set. |
Fix #8814.