Adding Event Rule Resource #150
Conversation
ghost
added
There was a problem hiding this comment.
Nice work @stmcallister 👌 I left a few comments but overall this looks great 👍
| err := json.Unmarshal([]byte(v), &obj) | ||
|
|
||
| if err != nil { | ||
| log.Printf(string(err.Error())) |
There was a problem hiding this comment.
What do you think about returning the error here if JSON unmarshal fails?
There was a problem hiding this comment.
For consistency, I've adopted the same strategy used in the extension resource. Would that work?
| func flattenSlice(v []interface{}) string { | ||
| b, err := json.Marshal(v) | ||
| if err != nil { | ||
| log.Printf(string(err.Error())) |
There was a problem hiding this comment.
Same as above, do we want to return the error here?
| if err != nil { | ||
| return err | ||
| } | ||
| for _, rule := range resp.EventRules { |
There was a problem hiding this comment.
What do you think about doing something like (not tested):
var found *pagerduty.EventRule
for _, rule := range resp.EventRules {
if rule.ID == d.Id() {
found = rule
break
}
}
if found == nil {
d.SetId("")
}
...
d.Set("catch_all", rule.CatchAll)
...That way you can set the attributes outside the loop 🙂
There was a problem hiding this comment.
This makes sense. I've adopted this idea in my next commit.
| } | ||
| } | ||
| // check if eventRule not found | ||
| if _, ok := d.GetOk("action_json"); !ok { |
There was a problem hiding this comment.
Does this work? This calls d.SetId("") if the response code was a 404.
I added an example above on how to clear it from the state if the event rule is missing from the list of event rules 🙂
There was a problem hiding this comment.
It appeared to have worked. 🤷♂ But, I've changed that logic to match what you were saying in the above comment.
There was a problem hiding this comment.
Nice work @stmcallister! 👌
Tests are passing and this LGTM 👍
This PR adds support for the Event Rules endpoint in the PagerDuty API. This relates to Issue 114
Acceptance Test Results