Remove status as it is not required #133
Conversation
If status is "warning" or "critical" Pagerduty API will reject as it only accepts "active" or "disabled".
ghost
added
the
| @@ -276,7 +276,6 @@ func resourcePagerDutyServiceRead(d *schema.ResourceData, meta interface{}) erro | |||
| } | |||
|
|
|||
| d.Set("name", service.Name) | |||
| d.Set("status", service.Status) | |||
There was a problem hiding this comment.
Removing this effectively renders the status field useless.
This should be kept to be able to read the current status of service.
I'd rather alter buildServiceStruct() to not set pagerduty.Service.status as it is a Computed field:
https://github.com/terraform-providers/terraform-provider-pagerduty/blob/9aca2da0cae321fd15db99d82bcaa6057e771e71/pagerduty/resource_pagerduty_service.go#L192
There was a problem hiding this comment.
Patrick,
the status for this matter is useless anyway, but I see your point and I agree that is better to remove it from buildServiceStruct, updated the PR
revert the READ function to have the status, but removed status from buildServiceStruct
There was a problem hiding this comment.
LGTM!
# git status
On branch fork-rodrigomgm-master
Your branch is up to date with 'fork-rodrigomgm/master'.
# git log -n 1
commit bb7509f47ffcba28370cdb8206b42760a4c35057 (HEAD -> fork-rodrigomgm-master, fork-rodrigomgm/master)
Author: Rodrigo Goncalves <rgoncalves@tyro.com>
Date: Tue May 21 10:44:17 2019 +1000
gofmt applied
# make testacc TESTARGS='-run=TestAccPagerDutyService_.* -count=1'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test "./pagerduty" -v -run=TestAccPagerDutyService_.* -count=1 -timeout 120m
=== RUN TestAccPagerDutyService_import
--- PASS: TestAccPagerDutyService_import (13.81s)
=== RUN TestAccPagerDutyService_Basic
--- PASS: TestAccPagerDutyService_Basic (25.75s)
=== RUN TestAccPagerDutyService_BasicWithIncidentUrgencyRules
--- PASS: TestAccPagerDutyService_BasicWithIncidentUrgencyRules (26.39s)
=== RUN TestAccPagerDutyService_FromBasicToCustomIncidentUrgencyRules
--- PASS: TestAccPagerDutyService_FromBasicToCustomIncidentUrgencyRules (19.00s)
=== RUN TestAccPagerDutyService_SupportHoursChange
--- PASS: TestAccPagerDutyService_SupportHoursChange (19.79s)
PASS
ok github.com/terraform-providers/terraform-provider-pagerduty/pagerduty 104.758s
There was a problem hiding this comment.
Hi @rodrigomgm,
sorry for the delay on this one. This LGTM 👍
If service status is "warning" or "critical" Pagerduty API will reject as it only accepts "active" or "disabled".
Having the Status as part of the payload will make cause the apply to fail if either "warning" or "critical" status. However not having it as part of the payload makes the Pagerduty accept the changes.