Add ability to load tasks/handlers from dir #1481

desa · 2017-07-17T13:35:01Z

Todo

Add batch tasks to example of mock directories.

Done

Load files from directory
Extend API to allow for new tickscript definition
Update Tickscript definition
Update Template var defition
Update Handler Definition

Todo

Add tests
Reload directory on OS signal
Clean up

Required for all non-trivial PRs

Rebased/mergable
Tests pass
CHANGELOG.md updated

desa · 2017-07-28T16:51:38Z

client/v1/client.go

@@ -203,8 +204,10 @@ type ExecutionStats struct {
 type TaskType int

 const (
-	StreamTask TaskType = 1
-	BatchTask  TaskType = 2
+	UndefinedTask TaskType = 0


Which of these do we need?

desa · 2017-07-28T16:51:50Z

client/v1/client.go

@@ -213,6 +216,10 @@ func (tt TaskType) MarshalText() ([]byte, error) {
 		return []byte("stream"), nil
 	case BatchTask:
 		return []byte("batch"), nil
+	case UndefinedTask:


desa · 2017-07-28T16:52:48Z

client/v1/client.go

@@ -2282,3 +2294,107 @@ func (d *Duration) UnmarshalText(data []byte) error {
 	*d = Duration(dur)
 	return nil
 }
+
+// TODO: this is also used in kapacitor/main.go
+type dbrps []DBRP


Export this and use in other package

desa · 2017-07-28T17:04:14Z

services/load/config.go

+	return nil
+}
+
+func (c Config) TasksDir() string {


probably doesn't need to be exported

desa · 2017-07-28T17:04:26Z

services/load/config.go

+	return filepath.Join(c.Dir, taskDir)
+}
+
+func (c Config) TemplatesDir() string {


desa · 2017-07-28T18:47:18Z

services/load/service.go

+
+// HandlerFiles gets a slice of all files with the .json, .yml, and
+// .yaml file extentions in the configured handler directory.
+func (s *Service) HandlerFiles() ([]string, error) {


desa · 2017-07-28T18:47:35Z

services/load/service.go

+	return handlers, nil
+}
+
+func (s *Service) Load() error {


Add logging

desa · 2017-07-28T18:47:57Z

services/load/service.go

+	return nil
+}
+
+func (s *Service) loadTickscripts() error {


change to load tasks

desa · 2017-07-28T18:48:07Z

services/load/service.go

+	return nil
+}
+
+func (s *Service) loadTickscript(f string) error {


desa · 2017-07-28T18:48:54Z

services/load/service.go

+}
+
+func (s *Service) loadTemplate(f string) error {
+	file, err := os.Open(f)


defer file.Close()

desa · 2017-07-31T15:33:14Z

@nathanielc This should be in a reviewable state. I still need to add a few more tests to the server package so that I can be completely confident in the changes, but it should reviewable.

nathanielc

This is looking great. I made a bunch of comments below, mostly addressing your TODO questions.

nathanielc · 2017-07-31T20:16:52Z

client/v1/client.go

@@ -2282,3 +2289,107 @@ func (d *Duration) UnmarshalText(data []byte) error {
 	*d = Duration(dur)
 	return nil
 }
+
+// TODO: this is also used in kapacitor/main.go


What does this TODO mean?

Old comment. Previously this code was in kapacitor/main.go and unexported. I moved it here and exported it. The TODO item was meant to remind me to update kapacitor/main.go to use this code instead.

nathanielc · 2017-07-31T20:19:12Z

cmd/kapacitor/main.go

+	if *dfile != "" {
+		f, err := os.Open(*dfile)
+		if err != nil {
+			return errors.Wrapf(err, "faild to open file %s", *dfile)


Typo faild -> failed

nathanielc · 2017-07-31T20:20:54Z

cmd/kapacitor/main.go

@@ -542,72 +541,15 @@ var (
 	dtype       = defineFlags.String("type", "", "The task type (stream|batch)")
 	dtemplate   = defineFlags.String("template", "", "Optional template ID")
 	dvars       = defineFlags.String("vars", "", "Optional path to a JSON vars file")
+	dfile       = defineFlags.String("file", "", "Optional path to a YAML or JSON vars file")


What's the difference between -vars and -file?

Its not clear from the help text.

nathanielc · 2017-07-31T20:22:53Z

cmd/kapacitor/main.go

 			ID:         id,
 			TemplateID: *dtemplate,
 			Type:       ttype,
 			DBRPs:      ddbrp,
 			TICKscript: script,
 			Vars:       vars,
 			Status:     client.Disabled,
-		})
+		}
+		if *dfile != "" {


Why isn't the logic if else here? Same question below for the update options logic

Not sure I totally follow. Are you saying to do

if *dfile != "" { o, err := fileVars.CreateTaskOptions() if err != nil { return err } _, err = cli.CreateTask(o) } else { o := client.CreateTaskOptions{ ID: id, TemplateID: *dtemplate, Type: ttype, DBRPs: ddbrp, TICKscript: script, Vars: vars, Status: client.Disabled, } _, err = cli.CreateTask(o) }

this instead?

nathanielc · 2017-07-31T20:23:19Z

cmd/kapacitor/main.go

@@ -822,7 +804,7 @@ func doDefineTemplate(args []string) error {
 }

 func defineTopicHandlerUsage() {
-	var u = `Usage: kapacitor define-topic-handler <topic id> <handler id> <path to handler spec file>
+	var u = `Usage: kapacitor define-topic-handler <path to handler spec file>


Yay! Now everything is in the handler file.

show-topic-handler and list topic-handlers handlers haven't changed. So they still require passing both the topic ID and handler ID as args correct?

nathanielc · 2017-07-31T20:48:46Z

services/load/config.go

+		return fmt.Errorf("directory %s must be contain subdirectory %s", c.Dir, handlerDir)
+	}
+
+	// TODO: we should probably check that we have the correct permissions to access the necessary files


No need to explicitly check permissions unless we want the files to be only user/group accessible for security purposes. Which is not the case here. So if we can read them great, if not we will report the error.

nathanielc · 2017-07-31T20:53:30Z

services/load/service.go

+	"github.com/pkg/errors"
+)
+
+var defaultURL = "http://localhost:9092"


OK here is a crazy idea.

We need to go through the API in order to get all the validation logic etc that is all there. But we do not need to actually make a network connection. Assuming that a localhost network connection will work is not valid in many environments, docker being a common one.

What if we simply created the HTTP request and then called ServerHTTP directly on the API server. This would be in effect the exact same thing but we never make and a network connection.

This may mean exposing the Client.Do method or something so that you can reuse the client.

I am thinking something like this

kapacitor/integrations/benchmark_test.go

Line 224 in d19fc26

httpdService.Handler.ServeHTTP(responseRecorder, writeRequest)

Doing it this way will eliminate a whole class of network bugs where we cannot reliably determine the connection credentials to ourselves. Thoughts?

Yeah I really like that.

nathanielc · 2017-07-31T20:55:19Z

services/load/service.go

+
+	handlersDir := s.config.handlersDir()
+
+	files, err := ioutil.ReadDir(handlersDir)


If this error is an error about the directory not existing silently ignore it. Meaning if the handlers dir does not exists, that is not an error but rather an indication that there are zero handlers to define.

in load/config.go the Validate method requires that the directory specified must have subdirectories tasks, templates, and handlers. Should I relax this requirement?

nathanielc · 2017-07-31T21:01:52Z

tick/ast/node.go

+}
+
+// TODO: Not sure how I feel about client.TaskType importing from client
+func (n *ProgramNode) DBRPs() []client.DBRP {


In general importing the client types in the server code makes sense as it ensures that the server and client are using the same types. But here we are not server code but rather an independent package that handles parsing scripts.

I recommend that you use the AST nodes as the public API from the tick package and move these methods into the server code where needed. So it would look something like dbrpsFromProgram(*ast.ProgramNode) []client.DBRP. Does that help?

nathanielc · 2017-07-31T21:03:37Z

tick/ast/node.go

@@ -981,6 +1024,101 @@ func newProgram(p position) *ProgramNode {
 	}
 }

+func NewProgramNodeFromTickscript(tickscript string) (*ProgramNode, error) {


I don't really like this function name, but I get that it is a common need to be able to type assert the ast.Node to its concrete type. Should we just have the Parse method return the concrete type?

FWIW The reasons I don't like the name is without reading the function's source I thought it was doing some kind of special operation to return a ProgramNode from a TICKscript instead of just using the Parse method.

Another option would be to move this method to the package where it is consumed as a helper method.

desa · 2017-08-02T22:49:31Z

cmd/kapacitord/main.go

-						}()
-						break Loop
+
+			case s := <-signalCh:


@nathanielc there has to be a better way than this, but I couldn't find an example. This could be my end-of-day brain not thinking straight. Thoughts?

desa · 2017-08-09T23:00:12Z

influxdata/chronograf#1721 will be able to remove the stream/batch button from tickscript UI when this is merged

desa · 2017-08-09T23:00:41Z

@121watts ^^

desa · 2017-08-23T14:31:54Z

Seems to be having an issue with batch tasks. Not exactly sure why. Will investigate.

F4ncyMooN · 2017-09-10T16:07:43Z

Hi, I'm looking for a convergence method for alerts.
For example, I want to monitor on 100 hosts which are located in 2 different DCs(50 in DC1, 50 in DC2), if 90% of DC1's hosts failed, only DC's alerts will be delivered.
I think this should be implemented by myself in some handlers.
Is it possible if I create some handlers without change the source code of kapacitor(just like udf) after this PR?

desa · 2017-09-11T16:13:35Z

@F4ncyMooN This PR is a little bit different. This PR introduces a way to load tasks and templates from a directory.

nathanielc

Overall this looks good. I just had a few code organization questions below.

nathanielc · 2017-09-11T16:42:39Z

cmd/kapacitor/main.go

-
-// Parse string of the form "db"."rp" where the quotes are optional but can include escaped quotes
-// within the strings.
-func (d *dbrps) Set(value string) error {


Why was this moved to the client package? The Set method is part of the flag.Value interface, which seems like it belongs here in the main package.

client.TaskVars ended up needing the same DBRP logic so rather than having two copies of the type, one in the main package and one in the client package I figured that I'd move it into client. I'm happy to have two separate types if you think thats best.

Why did the client.TaskVars need to implement the flag.Value interface?

Maybe the type in the client package should implement a TextUnmarshaler interface and then the type in the main package uses the type from the client package and adds the Set method to its own type? Does that work?

Yeah I like that.

@nathanielc Looks like implementing TextUnmarshaler causes issues with the regular JSON marshaling. Does having a method Unmarshal on the client.DBRP type work?

nathanielc · 2017-09-11T16:47:14Z

cmd/kapacitord/main.go

-			}()
+	Loop:
+		for {
+			select {


This select statement has only a single case, which means this can be simplified to for s := range signalCh {...}.

Hmm, ok so further down we check for a second signal. Maybe we should just keep a state about the signals we have received so its clear how transitions occur.

nathanielc · 2017-09-11T16:49:58Z

cmd/kapacitord/main.go

+						}
+					}
+
+					// This should never happen


What about os.Interrupt?

@nathanielc what should happen on os.Interrupt?

removing comment.

nathanielc · 2017-09-11T16:53:23Z

cmd/kapacitord/main.go

+					m.Logger.Println("I! SIGHUP received, Reloading tasks/templates/handlers directory...")
+					if err := cmd.Server.LoadService.Load(); err != nil {
+						m.Logger.Println(fmt.Sprintf("E! Failed to reload tasks/templates/handlers: %s", err))
+						if _, ok := err.(load.HardError); ok {


What other kinds of errors are returned? Should we push this logic into the server type? Maybe something in this method https://github.com/influxdata/kapacitor/blob/master/server/server.go#L882. Plus a method on the Server type to reload.

Not totally sure I follow. I think this logic might belong here. Load can return few different error types, but there's a configuration setting that specifies what should happen to the runtime in the case of an error. If the configuration says crash on error we wrap any errors coming from Load the error with a load.HardError.

nathanielc · 2017-09-11T16:54:05Z

cmd/kapacitord/run/command.go

@@ -127,6 +127,11 @@ func (cmd *Command) Run(args ...string) error {
 	if err := s.Open(); err != nil {
 		return fmt.Errorf("open server: %s", err)
 	}
+
+	if err := s.LoadService.Load(); err != nil {


Why isn't this part of the Open process for the Server?

desa · 2017-09-18T20:47:09Z

@nathanielc addressed most of the issues, only thing I was a bit unsure of was #1481 (comment)

nathanielc · 2017-09-18T20:50:39Z

cmd/kapacitord/main.go

+
+			case syscall.SIGHUP.String():
+				m.Diag.Info("SIGHUP received, reloading tasks/templates/handlers directory...")
+				if err := cmd.Server.LoadService.Load(); err != nil {


I was thinking, instead of doing cmd.Server.LoadService.Load() and handing the error types here in the main package, create a method on the server called Reload or something, that if it returns an error at all causes the Server to crash.
Then inside that method determine if the underlying errors returned from LoadService.Load are worth crashing. That way the main function crashes if it gets any error, and the Server contains the logic on what errors cause crashes.

Does that help?

desa · 2017-09-19T14:29:17Z

@nathanielc I think I addressed all the previous issues.

nathanielc · 2017-09-21T20:20:33Z

I am testing this out locally, here are some of the issues I ran into:

The kapacitor config command produces a load section with an empty directory, it would be good to apply a default dir like we do for the other dirs. See https://github.com/influxdata/kapacitor/blob/master/server/config.go#L181-L184
After configuring a directory Kapacitor now crashes on startup because the <dir>/tasks dir doesn't exist. Kapacitor should treat a lack of dir as there being no tasks configured, not as a condition to crash the server
The hard config option seems to not do anything different whether its on or off. Specifically I had a tick in the dir with some invalid syntax. NO matter what I could not get kapacitor to start without crashing.
When defining a handler I followed the example here and I got an error since the handler ID was missing from the file.
I changed the ID of a handler and the old handler is still around, it was not deleted. I expected the old handler to be removed
I renamed a task and now both the old and new tasks are defined. I expected the old task to be deleted.
At first I could not figure out how to create a task from a template. I originally put it in the tasks directory since its a task and just refers back to the template. Once I move the yaml file back to the template I got it working. I think tasks should exist in the tasks dir even if they refer to a template.
I expected an error if I forgot to define a template-id in a template task that already exists, but it doesn't error. It works, which means the files can drift from the real state since its possible to edit a file that would cause an error if created new, but otherwise not.

Those are the issues I ran into, mostly usability issues and the fact the the README in the examples dir is out of date so it lead me astray.

Its super nice to be able to define the tasks from dirs like this! Thanks!

desa · 2017-10-02T14:35:24Z

I am testing this out locally, here are some of the issues I ran into:

The kapacitor config command produces a load section with an empty directory, it would be good to apply a default dir like we do for the other dirs. See ?>https://github.com/influxdata/kapacitor/blob/master/server/config.go#L181-L184

Fixed

After configuring a directory Kapacitor now crashes on startup because the
/tasks dir doesn't exist. Kapacitor should treat a lack of dir as there being no tasks configured, not as a condition to crash the server

fixed.

At first I could not figure out how to create a task from a template. I originally put it in the tasks directory since its a task and just refers back to the template. Once I move the yaml file back to the template I got it working. I think tasks should exist in the tasks dir even if they refer to a template.

fixed

desa · 2017-10-02T14:35:45Z

@nathanielc

The hard config option seems to not do anything different whether its on or off. Specifically I had a tick in the dir with some invalid syntax. NO matter what I could not get kapacitor to start without crashing.

This was intentional. Hard errors only apply when re-loading when the process is running. The idea here was that if you’re starting launching from a directory that the tasks/templates/handlers should be treated as configuration and therefore the server should not start until they are valid. However if you’re running already and you make a change to your tasks/templates/handlers an error with one of those should not crash your server.

Happy to change this though.

I changed the ID of a handler and the old handler is still around, it was not deleted. I expected the old handler to be removed
I renamed a task and now both the old and new tasks are defined. I expected the old task to be deleted.

How do we do this? My current thought would be to have some kind of LoadTasks.lock file that will contain the tasks the were loading from a directory and then diff against that list. That seem reasonable?

When defining a handler I followed the example here and I got an error since the handler ID was missing from the file.
Those are the issues I ran into, mostly usability issues and the fact the the README in the examples dir is out of date so it lead me astray.

Which examples were/are problematic? When I use that directory as my load directory everything seems to work fine.

I expected an error if I forgot to define a template-id in a template task that already exists, but it doesn't error. It works, which means the files can drift from the real state since its possible to edit a file that would cause an error if created new, but otherwise not.

Should this be done by validating the file before even passing it to the API?

nathanielc · 2017-10-02T16:52:53Z

This was intentional. Hard errors only apply when re-loading when the process is running. The idea here was that if you’re starting launching from a directory that the tasks/templates/handlers should be treated as configuration and therefore the server should not start until they are valid. However if you’re running already and you make a change to your tasks/templates/handlers an error with one of those should not crash your server.

Hmm, I am beginning to think we should just remove the hard option and always have the reload log errors and never crash the server. This is how apache's httpd works. This way the processes doesn't assume any ownership of the workflow and lets the operator decided how to manage loading tasks from a dir. We should be however logging errors and incrementing a counter if the reload fails. This way it can be easily monitored.

How do we do this? My current thought would be to have some kind of LoadTasks.lock file that will contain the tasks the were loading from a directory and then diff against that list. That seem reasonable?

My though was track a list of tasks/templates/handlers that are created from the load service and then when the load service runs, if the file system no longer defines that task/template/handler then delete it. This list should be stored in the kapacitor.db file. It could be an explicit list or simply a new property on the data being stored for each item. Does that make sense?

Which examples were/are problematic? When I use that directory as my load directory everything seems to work fine.

I'll comment in-line on the issues I had.

Should this be done by validating the file before even passing it to the API?

No, I'd rather all validation logic live in the API, that way if someone isn't using the client they still get proper validation.

nathanielc

Added comments on the README that were confusing.

nathanielc · 2017-10-02T16:54:27Z

examples/load/README.md

+
+`dir` specifies the directory where the definition files exist.
+
+The specified directory should have three subdirectories `tasks`, `templates`, and `handlers`.


This line implies that the directories must exist, which is not the case.

nathanielc · 2017-10-02T16:55:08Z

examples/load/README.md

+The `tasks` directory will contain will contain task tickscripts.
+
+The `templates` directory will contain both template tickscripts and the associated templated task
+definition files (either yaml or json).


These lines need to be updated to reflect the new behavior.

nathanielc · 2017-10-02T16:55:34Z

examples/load/README.md

+* `id` - the file name without the tick extension
+* `type` - determined by introspection of the task (stream, batch)
+* `kind` - task, template. defined using a `set statement`
+* `subscriptions` - defined using the `subscribe` keyword followed by a specified subscription


Isn't the syntax dbrp now?

nathanielc · 2017-10-02T17:01:19Z

examples/load/README.md

+  "window": {"type" : "duration", "value" : "1m" },
+  "slack_channel": {"type" : "string", "value" : "#alerts_testing" }
+}
+```


This is technically correct yaml but quite non-standard a confusing, this reads more like yaml

dbrp: - telegraf.autogen - telegraf.not_autogen template: base_template vars: measurement: type: string value: cpu where_filter: type: lambda value: "\"cpu\" == 'cpu-total'" groups: type: list value: - type: string value: host - type: string value: dc field: type: string value : usage_idle warn: type: lambda value: "\"mean\" < 30.0" crit: type: lambda value: "\"mean\" < 10.0" window: type: duration value : 1m slack_channel: type: string value: "#alerts_testing"

nathanielc · 2017-10-02T17:01:56Z

examples/load/README.md

+  channel: '#alerts'
+```
+
+the name of the file will be used as the handler id.


This is false, the id is read from the file and the filename is not used.

desa · 2017-10-03T13:20:05Z

@nathanielc removed the hard error condition and added the logic for deleting stale tasks/templates/handlers that were loaded from a directory. The todo items I have left are

Add tests for DAO
Make edits to README
Cleanup remove logic a little

desa · 2017-10-06T15:38:51Z

@nathanielc added tests. Should be ready for final review

nathanielc

Added some comments.

nathanielc · 2017-10-06T16:08:49Z

examples/load/README.md

+
+`dir` specifies the directory where the definition files exist.
+
+The specified directory will check to see if any of three subdirectories `tasks`, `templates`, and `handlers` exists.


The configured dir may contain any of these sub-directories: tasks, templates, and handlers.

nathanielc · 2017-10-06T16:09:23Z

examples/load/README.md

+The `tasks` directory will contain will contain task tickscripts and and the associated templated task
+definition files (either yaml or json).
+
+The `templates` directory will contain both template tickscripts


The templates directory will contain template tickscripts

nathanielc · 2017-10-06T16:09:41Z

examples/load/README.md

+
+The `templates` directory will contain both template tickscripts
+
+The `handlers` directory will contain will contain all topic handler definitions in yaml or json.


The handlers directory will contain will contain topic handler definitions in yaml or json.

nathanielc · 2017-10-06T16:10:12Z

examples/load/README.md

+```
+dbrp "telegraf"."autogen"
+
+var measurement string


This example is actually a template so it can't be placed in the tasks dir.

nathanielc · 2017-10-06T16:10:39Z

examples/load/README.md

+dbrp:
+  - telegraf.autogen
+  - telegraf.not_autogen
+template: base_template


This should be templated-id

nathanielc · 2017-10-06T16:14:01Z

examples/load/README.md

+
+### Templated Tasks
+
+Template variables may be added as either json or yaml.


I think the README should have a simple TICKscript above to demonstrate creating tasks.

Then in this Template section it should first define a TICKscript template. Then show how that template can be referenced from a file in the tasks directory.

We want to make it clear that anything in the tasks dir will create a task, and anything in the templates dir will create a template.

Added a section on defining a template before and the templated tasks now use that template. Think that should be sufficient? Or did you have something else in mind?

nathanielc · 2017-10-06T16:15:56Z

server/server.go

@@ -896,7 +896,7 @@ func (s *Server) Open() error {
 	}

 	if err := s.LoadService.Load(); err != nil {
-		return fmt.Errorf("loading tasks/templates/handlers: %s", err)


I think its fine for the server to refuse to start if it has bad configuration, but we do not want it to crash from a simple typo.

not sure exactly what to do with this. Should I keep the error here and do some checking of the type of error, or just leave it as is?

What types of errors would you check for? Seems like if it errors at all it should return the error.

nathanielc · 2017-10-06T16:16:22Z

server/server.go

@@ -1060,7 +1060,7 @@ func (s *Server) writeID(file string, id uuid.UUID) error {
 func (s *Server) Reload() error {


If this function can't error anymore we should remove the error return value and simplify the caller to not need to handle the error.

nathanielc

Looks good. Testing locally it works well.

Can you update the bash completion for the CLI changes?

nathanielc · 2017-10-06T19:02:28Z

services/load/service.go

+}
+
+func (s *Service) loadedTasks() ([]string, error) {
+	items, err := s.items.List("tasks")


We should create constants for the tasks , templates and handlers strings, since they are part of the key for the DOA. A typo would corrupt the data :(.

nathanielc · 2017-10-06T19:05:39Z

services/load/service.go

+
+func NewService(c Config, h http.Handler, d Diagnostic) (*Service, error) {
+	cfg := client.Config{
+		URL: defaultURL,


Can we add a unique user agent here so its clear the consumer of the API is this internal client from Load?

desa · 2017-10-06T20:00:44Z

@nathanielc I think I got the completion right, but I'm not totally sure. I sometimes have issues with it on OSX.

Does this seem right?

396ab78

Add check for tickscript defined dbrps Fix error that allowed you to set dbrps twice Add template and task dbrp via tickscript Remove dao test file Fix issue with updating task without dbrp values Add new template vars structure Load tasks and templates from directory Update define-topic-handler subcommand to 1 arg Add loadHandlers method to load service Add tests and prevent certain updates to templates Update parser to define dbrp as reference.referenc Fix issue where tasks weren't reloaded Replace correct topic handler Listen for SIGHUP to reload tasks/templates/handld Move dbrps type to client package Unexport internal functions Address todo items Make assorted changes suggested in PR Add test for dbrp Add tests for node task type and dbrps Fix comment Use ServeHTTP instead of making actual http req Add soft/hard configuration option Fix typos and remove TODO comments Don't error if the directories are missing Append load service Move util methods into task_store package Improve error messaging Clean up kapacitor and kapacitord mains Remove old TODO comment Add error check in doDefine Add load example directory Address issues from PR Add diagnostic to load service Add batch examples and tests Add reload method to server Add dbrp type to kapacitor main package Use explicit specification for dbrp in files Set sane default for demo config Skip load if directory doesn't exist Unwrap error so that correct error type Move templated tasks to task dir from templates Remove commented out code Remove hard error Add error stat to load service Delete tasks/templates/hanlders when appropriate Delete tasks/template/handlers when appropraite Cleanup example of load directory Add Error to load diagnostic Check for nil storage service Clean-up load functions Add test for load service Add test for topic-handlers Prevent server crash on error Fix wording in README Remove error from Reload function Make additional changes to README Crash on initial Load Add more to readme Add UserAgent and task string constants Update define-topic-handler completion

desa · 2017-10-06T20:36:22Z

@nathanielc just rebased. I think things should be good to go now.

desa added the in progress label Jul 17, 2017

desa force-pushed the md-issue#1313 branch 2 times, most recently from 27c8b04 to 1940364 Compare July 21, 2017 18:59

desa commented Jul 28, 2017

View reviewed changes

desa requested review from mark-rushakoff and nathanielc and removed request for mark-rushakoff July 31, 2017 15:31

nathanielc reviewed Jul 31, 2017

View reviewed changes

desa commented Aug 2, 2017

View reviewed changes

desa force-pushed the md-issue#1313 branch from 54620eb to dd5602b Compare August 7, 2017 23:16

desa changed the title ~~WIP: Add ability to load tasks/handlers from dir~~ Add ability to load tasks/handlers from dir Aug 8, 2017

desa force-pushed the md-issue#1313 branch from dd5602b to fa913db Compare August 23, 2017 14:52

nathanielc suggested changes Sep 11, 2017

View reviewed changes

desa force-pushed the md-issue#1313 branch from 4937470 to 7472b35 Compare September 18, 2017 18:54

nathanielc reviewed Sep 18, 2017

View reviewed changes

ghost assigned desa Oct 2, 2017

desa force-pushed the md-issue#1313 branch from c46f72d to 5a5ce59 Compare October 2, 2017 14:59

nathanielc reviewed Oct 2, 2017

View reviewed changes

nathanielc reviewed Oct 6, 2017

View reviewed changes

desa force-pushed the md-issue#1313 branch from 396ab78 to 1de435d Compare October 6, 2017 20:34

nathanielc approved these changes Oct 9, 2017

View reviewed changes

desa merged commit 0a8abec into master Oct 9, 2017

desa deleted the md-issue#1313 branch October 9, 2017 17:01

ghost removed the in progress label Oct 9, 2017

desa mentioned this pull request Oct 23, 2017

[Feature Request] Kapacitor needs a way to automatically load tick scripts from a directory. #1313

Closed


		handlersDir := s.config.handlersDir()

		files, err := ioutil.ReadDir(handlersDir)


		`dir` specifies the directory where the definition files exist.

		The specified directory should have three subdirectories `tasks`, `templates`, and `handlers`.


		`dir` specifies the directory where the definition files exist.

		The specified directory will check to see if any of three subdirectories `tasks`, `templates`, and `handlers` exists.


		The `templates` directory will contain both template tickscripts

		The `handlers` directory will contain will contain all topic handler definitions in yaml or json.


		### Templated Tasks

		Template variables may be added as either json or yaml.

		@@ -1060,7 +1060,7 @@ func (s *Server) writeID(file string, id uuid.UUID) error {
		func (s *Server) Reload() error {

Add ability to load tasks/handlers from dir #1481

Add ability to load tasks/handlers from dir #1481

Conversation

desa commented Jul 17, 2017 • edited Loading

Todo

Done

Todo

Required for all non-trivial PRs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa commented Jul 31, 2017

nathanielc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa Aug 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa commented Aug 9, 2017

desa commented Aug 9, 2017

desa commented Aug 23, 2017

F4ncyMooN commented Sep 10, 2017

desa commented Sep 11, 2017

nathanielc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa commented Sep 18, 2017

nathanielc Sep 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa commented Sep 19, 2017

nathanielc commented Sep 21, 2017

desa commented Oct 2, 2017 • edited Loading

desa commented Oct 2, 2017 • edited Loading

nathanielc commented Oct 2, 2017

nathanielc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa commented Oct 3, 2017

desa commented Oct 6, 2017 • edited Loading

desa commented Jul 17, 2017 •

edited

Loading

desa Aug 1, 2017 •

edited

Loading

nathanielc Sep 18, 2017 •

edited

Loading

desa commented Oct 2, 2017 •

edited

Loading

desa commented Oct 2, 2017 •

edited

Loading

desa commented Oct 6, 2017 •

edited

Loading