Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch to allow services' status to be managed by the provider #252

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qhua948
Copy link

@qhua948 qhua948 commented Jul 16, 2020

No description provided.

@jjm
Copy link
Contributor

jjm commented Aug 17, 2020

I was wondering if you have managed to confirm that services are created in an disabled state? Thing is I've got a long running support case where setting a state of disabled on a newly created service has no effect and it's just created in a active state (I'm told this is the correct behaviour).

From my understanding form our own tooling (not Terraform) is that the service would only end up being disabled after the a second apply, unless it confirms the service is in the correct state (which is the workaround I've reached).

@qhua948
Copy link
Author

qhua948 commented Aug 19, 2020

I was wondering if you have managed to confirm that services are created in an disabled state? Thing is I've got a long running support case where setting a state of disabled on a newly created service has no effect and it's just created in a active state (I'm told this is the correct behaviour).

From my understanding form our own tooling (not Terraform) is that the service would only end up being disabled after the a second apply, unless it confirms the service is in the correct state (which is the workaround I've reached).

I think I did test it with creating a disabled service.

I have since lost access to pagerduty's API, would appreciate if you could test it out as well.

@jjm
Copy link
Contributor

jjm commented Sep 22, 2020

I've not done much GoLang before, but my support case has been updated and the API docs been updated with the following

If status is included in the request, it must have a value of active when creating a new service. If a different status is required, make a second request to update the service.

(taken from from https://developer.pagerduty.com/api-reference/reference/REST/openapiv3.json/paths/~1services/post)

They are looking at making a request with a status of disabled result in a 400 error. So this PR would beed to create the service as active and then set the status to disabled.

@stmcallister stmcallister self-requested a review February 26, 2021 21:32
@stmcallister
Copy link
Contributor

stmcallister commented Feb 26, 2021

@qhua948 Thanks for adding this! I didn't even know about the status field until I saw this 😅 In general everything looks good. However, the test is failing with the following message.

TF_ACC=1 go test -run "TestAccPagerDutyService_WithStatus" ./pagerduty -v -timeout 120m 
=== RUN   TestAccPagerDutyService_WithStatus
    testing.go:654: Step 1 error: Check failed: Check 3/10 error: pagerduty_service.foo: Attribute 'description' expected "bar", got "foo"
--- FAIL: TestAccPagerDutyService_WithStatus (10.80s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-pagerduty/pagerduty	11.413s
FAIL

If you can fix this I can merge. Also, sorry for the delaying in taking a look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants