Improve consistency by using response from POST/PUT requests directly to save state #901
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -559,41 +559,8 @@ func resourceDatadogMonitorCreate(d *schema.ResourceData, meta interface{}) erro | |||
| return resourceDatadogMonitorRead(d, meta) | |||
| } | |||
|
|
|||
| func resourceDatadogMonitorRead(d *schema.ResourceData, meta interface{}) error { | |||
| func updateMonitorState(d *schema.ResourceData, meta interface{}, m *datadogV1.Monitor) error { | |||
| providerConf := meta.(*ProviderConfiguration) | |||
There was a problem hiding this comment.
is providerConf still used in this method?
There was a problem hiding this comment.
yes, to get the time near the end
| d.Set("type", m.GetType()) | ||
| d.Set("priority", m.GetPriority()) | ||
| d.Set("restricted_roles", m.GetRestrictedRoles()) | ||
| if err := d.Set("name", m.GetName()); err != nil { |
There was a problem hiding this comment.
Does this error if the value being set is empty, or is it just if an internal TF SDK piece fails?
There was a problem hiding this comment.
It errors if an internal SDK piece fails, like wrong type or something
| if err := d.Set("tags", tags); err != nil { | ||
| return err | ||
| } | ||
| // TODO Is this one of those options that we neeed to check? |
There was a problem hiding this comment.
Do we know what this TODO is for?
| @@ -771,12 +822,13 @@ func resourceDatadogMonitorUpdate(d *schema.ResourceData, meta interface{}) erro | |||
| delete(silencedList, scope) | |||
| } | |||
| } | |||
| if _, _, err = datadogClientV1.MonitorsApi.UpdateMonitor(authV1, i).Body(*m).Execute(); err != nil { | |||
| monitorResp, _, err = datadogClientV1.MonitorsApi.UpdateMonitor(authV1, i).Body(*m).Execute() | |||
There was a problem hiding this comment.
Perhaps you have already (not sure if we have an explicit test for this) but can you confirm we get the full monitor back here when we unmute scopes?
There was a problem hiding this comment.
It's still the same update call, so I'd say yes, and monitors team said I could rely on output of PUT request
There was a problem hiding this comment.
(also yeah we have a specific unmute test)
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Avoid consistency issues, due to replication delays, caused by calling the GET endpoint right after POST/PUT
Also fixes some linting warnings, and reads error returned by
d.Set