Downtime improvements #204
Conversation
platinummonkey
commented
May 15, 2019
platinummonkey
commented
- Adds the timezone parameter
- Adds descriptions to parameters
- Adds validation to monitor_tags
- Ensures that the updated downtime id is correctly handled for future immutable downtimes
Adds the timezone parameter Adds descriptions to parameters Adds validation to monitor_tags Ensures that the updated downtime id is correctly handled for future immutable downtimes
ghost
added
the
|
cc @bkabrda |
… supported on lists or sets"
There was a problem hiding this comment.
Overall looks good, thanks for this. Left two quick comments/questions.
Can you also add the new information to the docs - https://github.com/terraform-providers/terraform-provider-datadog/blob/master/website/docs/r/monitor.html.markdown#argument-reference 🙇
| Optional: true, | ||
| Type: schema.TypeInt, | ||
| Optional: true, | ||
| ConflictsWith: []string{"monitor_tags"}, |
There was a problem hiding this comment.
Is setting both not an available option here? Would the API return an error previously if both were being sent?
There was a problem hiding this comment.
while technically it works, it's one of those, it's broken if the tags also don't match the monitor and won't apply it'll be an API validation in the future.
The matching logic today is: (pseudo-python): if (downtime.monitor_id is None or downtime.monitor_id == monitor.id) and monitor.tags.issuperset(downtime.monitor_tags)
in the case of no downtime.monitor_tags it's an empty set, so it'll always match. the downtime.monitor_tags was meant to match multiple monitors that have those tags.
There was a problem hiding this comment.
Ok makes sense, thanks for the explanation :)
| @@ -305,6 +326,8 @@ func resourceDatadogDowntimeUpdate(d *schema.ResourceData, meta interface{}) err | |||
| if err = client.UpdateDowntime(dt); err != nil { | |||
| return fmt.Errorf("error updating downtime: %s", err.Error()) | |||
| } | |||
| // handle the case when a downtime is replaced | |||
| d.SetId(strconv.Itoa(dt.GetId())) | |||
There was a problem hiding this comment.
Just to confirm, the idea here being that we would be updating the current downtime on each subsequent apply, rather than creating a new one?
There was a problem hiding this comment.
It just ensures the downtime ID is set. In the future when downtimes are immutable this ID can change on updates
Downtime improvements
