Enable/Disable by glob

[yosiat]

Issue - #337

• Add FindTasks method for fetching all rawTask[] by a predicate
• Enabling/Disabling by glob

For example:
> kapacitor enable "window_*"
Will enable all tasks that starts with "window_"
The same for disable

Notes:

• I think this would be nice for delete as well
• The enable is not efficient as possible - I am accessing boltdb for each task, I can change it, but I think it will hurt the readability
• If at least one task is failed to enable/disable I am stopping and returning an error - are we ok about this behaviour?

[nathanielc]

I think this would be nice for delete as well

+1

The enable is not efficient as possible - I am accessing boltdb for each task, I can change it, but I think it will hurt the readability

Simple is better

If at least one task is failed to enable/disable I am stopping and returning an error - are we ok about this behavior?

Yes

[nathanielc]

If we are not doing any matching here why are we not looping over LoadRaw?

[yosiat]

Umm.. I agree, but the reason we need to handle the case where tasks is empty, and we want to fetch all tasks.

The looping LoadRaw is more simple & elegant.. do you have any way can do this and still support the empty tasks array?

[nathanielc]

I may be missing it but I do not see the change in behavior from the previous implementation to this one?

[yosiat]

Let's make sure we are on the same idea, you are suggesting that instead of using FindTasks and looking at the bucket we will do something like this:

for taskName := range tasks {
   raw:=ts.LoadRaw(taskName)
  // convert to TaskSummaryInfo, and IsEnabled, and ExecutionStatus
}

If you are suggesting doing this approach, we have the case where the tasks are empty and we want to fetch all tasks - "/tasks", so we are using FindTasks to find the matching tasks.

[nathanielc]

I am not suggesting that, but rather looking at the original implementation it handled the case where tasks was empty via the ForEach boltdb bucket call. I see that method has changed but I do not see how its behavior has changed so I am confused as to why it was modified. I am probably just missing something....

[yosiat]

ok, you are suggesting to undo the change in this function? I changed this function to use the FindTasks, but after a little bit of thinking it makes things harder.

[nathanielc]

Yes, there is no need to use the FindTasks if it is not necessary.

[yosiat]

@nathanielc are we ok on this PR or do you want me to implement delete by glob as well before we merge?

[nathanielc]

@yosiat This looks good to go. Do you want to add a CHANGELOG entry real quick?

[yosiat]

I will do in one hour, I forgot to update the usage for the relevant
commands in kapacitor cmd
On Apr 1, 2016 8:27 PM, "Nathaniel Cook" [email protected] wrote:

@yosiat https://github.com/yosiat This looks good to go. Do you want to
add a CHANGELOG entry real quick?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#381 (comment)

[yosiat]

@nathanielc Updated and added help usage..
please make sure it's ok, and I will squash the commits

[nathanielc]

These lines look like there are extras?

[yosiat]

Oops.. I did copy/paste, Will fix this.

[nathanielc]

@yosiat Looks good, I'll merge when its squashed

[yosiat]

@nathanielc squashed

[devfacet]

@yosiat : does it mean kapacitor enable "*" and kapacitor disable "*" enabled and disable all the tasks?

[yosiat]

@fatihcode yes