Skip to content

Commit

Permalink
allow ID for task and template to be updated (influxdata#618)
Browse files Browse the repository at this point in the history
  • Loading branch information
Nathaniel Cook committed Jun 6, 2016
1 parent 80d4e71 commit 6091de6
Show file tree
Hide file tree
Showing 9 changed files with 603 additions and 304 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ batch
- [#553](https://github.com/influxdata/kapacitor/issues/553): Periodically check if new InfluxDB DBRPs have been created.
- [#602](https://github.com/influxdata/kapacitor/issues/602): Fix missing To property on email alert handler.
- [#581](https://github.com/influxdata/kapacitor/issues/581): Record/Replay batch tasks get cluster info from task not API.
- [#613](https://github.com/influxdata/kapacitor/issues/613): BREAKING: Allow the ID of templates and tasks to be updated via the PATCH method.
The breaking change is that now PATCH request return a 200 with the template or task definition, where before they returned 204.

## v0.13.1 [2016-05-13]

Expand Down
2 changes: 0 additions & 2 deletions client/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ Response with task id and link.
| Code | Meaning |
| ---- | ------- |
| 200 | Task created, contains task information. |
| 204 | Task updated, no content |
| 404 | Task does not exist |

### Get Task
Expand Down Expand Up @@ -587,7 +586,6 @@ PATCH /kapacitor/v1/templates/TEMPLATE_ID
| Code | Meaning |
| ---- | ------- |
| 200 | Template created, contains template information. |
| 204 | Template updated, no content |
| 404 | Template does not exist |

### Get Template
Expand Down
32 changes: 18 additions & 14 deletions client/v1/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ func (c *Client) CreateTask(opt CreateTaskOptions) (Task, error) {
}

type UpdateTaskOptions struct {
ID string `json:"id,omitempty"`
TemplateID string `json:"template-id,omitempty"`
Type TaskType `json:"type,omitempty"`
DBRPs []DBRP `json:"dbrps,omitempty"`
Expand All @@ -661,31 +662,32 @@ type UpdateTaskOptions struct {

// Update an existing task.
// Only fields that are not their default value will be updated.
func (c *Client) UpdateTask(link Link, opt UpdateTaskOptions) error {
func (c *Client) UpdateTask(link Link, opt UpdateTaskOptions) (Task, error) {
t := Task{}
if link.Href == "" {
return fmt.Errorf("invalid link %v", link)
return t, fmt.Errorf("invalid link %v", link)
}

var buf bytes.Buffer
enc := json.NewEncoder(&buf)
err := enc.Encode(opt)
if err != nil {
return err
return t, err
}

u := *c.url
u.Path = link.Href

req, err := http.NewRequest("PATCH", u.String(), &buf)
if err != nil {
return err
return t, err
}

_, err = c.do(req, nil, http.StatusNoContent)
_, err = c.do(req, &t, http.StatusOK)
if err != nil {
return err
return t, err
}
return nil
return t, nil
}

type TaskOptions struct {
Expand Down Expand Up @@ -860,37 +862,39 @@ func (c *Client) CreateTemplate(opt CreateTemplateOptions) (Template, error) {
}

type UpdateTemplateOptions struct {
ID string `json:"id,omitempty"`
Type TaskType `json:"type,omitempty"`
TICKscript string `json:"script,omitempty"`
}

// Update an existing template.
// Only fields that are not their default value will be updated.
func (c *Client) UpdateTemplate(link Link, opt UpdateTemplateOptions) error {
func (c *Client) UpdateTemplate(link Link, opt UpdateTemplateOptions) (Template, error) {
t := Template{}
if link.Href == "" {
return fmt.Errorf("invalid link %v", link)
return t, fmt.Errorf("invalid link %v", link)
}

var buf bytes.Buffer
enc := json.NewEncoder(&buf)
err := enc.Encode(opt)
if err != nil {
return err
return t, err
}

u := *c.url
u.Path = link.Href

req, err := http.NewRequest("PATCH", u.String(), &buf)
if err != nil {
return err
return t, err
}

_, err = c.do(req, nil, http.StatusNoContent)
_, err = c.do(req, &t, http.StatusOK)
if err != nil {
return err
return t, err
}
return nil
return t, nil
}

type TemplateOptions struct {
Expand Down
53 changes: 39 additions & 14 deletions client/v1/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func Test_ReportsErrors(t *testing.T) {
{
name: "UpdateTask",
fnc: func(c *client.Client) error {
err := c.UpdateTask(c.TaskLink(""), client.UpdateTaskOptions{})
_, err := c.UpdateTask(c.TaskLink(""), client.UpdateTaskOptions{})
return err
},
},
Expand Down Expand Up @@ -89,7 +89,7 @@ func Test_ReportsErrors(t *testing.T) {
{
name: "UpdateTemplate",
fnc: func(c *client.Client) error {
err := c.UpdateTemplate(c.TemplateLink(""), client.UpdateTemplateOptions{})
_, err := c.UpdateTemplate(c.TemplateLink(""), client.UpdateTemplateOptions{})
return err
},
},
Expand Down Expand Up @@ -516,7 +516,8 @@ func Test_UpdateTask(t *testing.T) {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, "unexpected UpdateTask body: got:\n%v\nexp:\n%v\n", task, exp)
} else {
w.WriteHeader(http.StatusNoContent)
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, `{"link": {"rel":"self", "href":"/kapacitor/v1/tasks/taskname"}, "id":"taskname"}`)
}
} else {
w.WriteHeader(http.StatusBadRequest)
Expand All @@ -528,7 +529,7 @@ func Test_UpdateTask(t *testing.T) {
}
defer s.Close()

err = c.UpdateTask(
task, err := c.UpdateTask(
c.TaskLink("taskname"),
client.UpdateTaskOptions{
DBRPs: []client.DBRP{{Database: "newdb", RetentionPolicy: "rpname"}},
Expand All @@ -547,6 +548,9 @@ func Test_UpdateTask(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if got, exp := task.Link.Href, "/kapacitor/v1/tasks/taskname"; got != exp {
t.Errorf("unexpected link.Href got %s exp %s", got, exp)
}
}

func Test_UpdateTask_Enable(t *testing.T) {
Expand All @@ -563,7 +567,8 @@ func Test_UpdateTask_Enable(t *testing.T) {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, "unexpected UpdateTask body: got:\n%v\nexp:\n%v\n", task, exp)
} else {
w.WriteHeader(http.StatusNoContent)
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, `{"link": {"rel":"self", "href":"/kapacitor/v1/tasks/taskname"}, "id":"taskname", "status": "enabled"}`)
}
} else {
w.WriteHeader(http.StatusBadRequest)
Expand All @@ -575,7 +580,7 @@ func Test_UpdateTask_Enable(t *testing.T) {
}
defer s.Close()

err = c.UpdateTask(
task, err := c.UpdateTask(
c.TaskLink("taskname"),
client.UpdateTaskOptions{
Status: client.Enabled,
Expand All @@ -584,6 +589,12 @@ func Test_UpdateTask_Enable(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if got, exp := task.Link.Href, "/kapacitor/v1/tasks/taskname"; got != exp {
t.Errorf("unexpected link.Href got %s exp %s", got, exp)
}
if got, exp := task.Status, client.Enabled; got != exp {
t.Errorf("unexpected task.Status got %s exp %s", got, exp)
}
}

func Test_UpdateTask_Disable(t *testing.T) {
Expand All @@ -601,7 +612,8 @@ func Test_UpdateTask_Disable(t *testing.T) {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, "unexpected UpdateTask body: got:\n%v\nexp:\n%v\n", task, exp)
} else {
w.WriteHeader(http.StatusNoContent)
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, `{"link": {"rel":"self", "href":"/kapacitor/v1/tasks/taskname"}, "id":"taskname", "status": "disabled"}`)
}
} else {
w.WriteHeader(http.StatusBadRequest)
Expand All @@ -613,14 +625,20 @@ func Test_UpdateTask_Disable(t *testing.T) {
}
defer s.Close()

err = c.UpdateTask(
task, err := c.UpdateTask(
c.TaskLink("taskname"),
client.UpdateTaskOptions{
Status: client.Disabled,
})
if err != nil {
t.Fatal(err)
}
if got, exp := task.Link.Href, "/kapacitor/v1/tasks/taskname"; got != exp {
t.Errorf("unexpected link.Href got %s exp %s", got, exp)
}
if got, exp := task.Status, client.Disabled; got != exp {
t.Errorf("unexpected task.Status got %s exp %s", got, exp)
}
}

func Test_DeleteTask(t *testing.T) {
Expand Down Expand Up @@ -990,15 +1008,15 @@ func Test_CreateTemplate(t *testing.T) {
Type: client.StreamTask,
TICKscript: tickScript,
})
if got, exp := string(template.Link.Href), "/kapacitor/v1/templates/templatename"; got != exp {
if err != nil {
t.Fatal(err)
}
if got, exp := template.Link.Href, "/kapacitor/v1/templates/templatename"; got != exp {
t.Errorf("unexpected template link got %s exp %s", got, exp)
}
if got, exp := template.ID, "templatename"; got != exp {
t.Errorf("unexpected template ID got %s exp %s", got, exp)
}
if err != nil {
t.Fatal(err)
}
}

func Test_UpdateTemplate(t *testing.T) {
Expand All @@ -1015,7 +1033,8 @@ func Test_UpdateTemplate(t *testing.T) {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, "unexpected UpdateTemplate body: got:\n%v\nexp:\n%v\n", template, exp)
} else {
w.WriteHeader(http.StatusNoContent)
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, `{"link": {"rel":"self", "href":"/kapacitor/v1/templates/templatename"}, "id":"templatename"}`)
}
} else {
w.WriteHeader(http.StatusBadRequest)
Expand All @@ -1027,7 +1046,7 @@ func Test_UpdateTemplate(t *testing.T) {
}
defer s.Close()

err = c.UpdateTemplate(
template, err := c.UpdateTemplate(
c.TemplateLink("templatename"),
client.UpdateTemplateOptions{
Type: client.BatchTask,
Expand All @@ -1036,6 +1055,12 @@ func Test_UpdateTemplate(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if got, exp := template.Link.Href, "/kapacitor/v1/templates/templatename"; got != exp {
t.Errorf("unexpected template link got %s exp %s", got, exp)
}
if got, exp := template.ID, "templatename"; got != exp {
t.Errorf("unexpected template ID got %s exp %s", got, exp)
}
}

func Test_DeleteTemplate(t *testing.T) {
Expand Down
8 changes: 6 additions & 2 deletions client/v1/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ paths:
schema:
$ref: '#/definitions/Task'
responses:
'204':
'200':
description: Update succeeded
schema:
$ref: '#/definitions/Task'
default:
description: A processing or an unexpected error.
schema:
Expand Down Expand Up @@ -233,8 +235,10 @@ paths:
schema:
$ref: '#/definitions/Template'
responses:
'204':
'200':
description: Update succeeded
schema:
$ref: '#/definitions/Template'
default:
description: A processing or an unexpected error.
schema:
Expand Down
12 changes: 6 additions & 6 deletions cmd/kapacitor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ func doDefine(args []string) error {
Status: client.Disabled,
})
} else {
err = cli.UpdateTask(
_, err = cli.UpdateTask(
l,
client.UpdateTaskOptions{
TemplateID: *dtemplate,
Expand All @@ -668,11 +668,11 @@ func doDefine(args []string) error {
}

if !*dnoReload && task.Status == client.Enabled {
err := cli.UpdateTask(l, client.UpdateTaskOptions{Status: client.Disabled})
_, err := cli.UpdateTask(l, client.UpdateTaskOptions{Status: client.Disabled})
if err != nil {
return err
}
err = cli.UpdateTask(l, client.UpdateTaskOptions{Status: client.Enabled})
_, err = cli.UpdateTask(l, client.UpdateTaskOptions{Status: client.Enabled})
if err != nil {
return err
}
Expand Down Expand Up @@ -761,7 +761,7 @@ func doDefineTemplate(args []string) error {
TICKscript: script,
})
} else {
err = cli.UpdateTemplate(
_, err = cli.UpdateTemplate(
l,
client.UpdateTemplateOptions{
Type: ttype,
Expand Down Expand Up @@ -1070,7 +1070,7 @@ func doEnable(args []string) error {
return errors.Wrap(err, "listing tasks")
}
for _, task := range tasks {
err := cli.UpdateTask(
_, err := cli.UpdateTask(
task.Link,
client.UpdateTaskOptions{Status: client.Enabled},
)
Expand Down Expand Up @@ -1128,7 +1128,7 @@ func doDisable(args []string) error {
return errors.Wrap(err, "listing tasks")
}
for _, task := range tasks {
err := cli.UpdateTask(
_, err := cli.UpdateTask(
task.Link,
client.UpdateTaskOptions{Status: client.Disabled},
)
Expand Down
Loading

0 comments on commit 6091de6

Please sign in to comment.