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

Instantiate scheduled_actions if incident_urgency_rules with type = "use_support_hours" #99

Merged
merged 4 commits into from
Nov 2, 2018
Merged

Conversation

sarkis
Copy link
Contributor

@sarkis sarkis commented Oct 27, 2018

This fixes #97, instantiating scheduled actions if use_support_hours incident urgency type is set. It ensures that the pagerduty API calls will include an empty scheduled_actions in certain cases, explained better by the bug report in #97.

@ghost ghost added the size/XS label Oct 27, 2018
@sarkis sarkis closed this Oct 27, 2018
@sarkis sarkis reopened this Oct 27, 2018
@sarkis sarkis changed the title WIP: properly send an empty scheduled actions for special cases Instantiate scheduled_actions if incident_urgency_rules with type = "use_support_hours" Oct 27, 2018
@ghost ghost added size/M and removed size/XS labels Oct 27, 2018
@sarkis
Copy link
Contributor Author

sarkis commented Oct 27, 2018

Test added and ready for a review.

@sarkis
Copy link
Contributor Author

sarkis commented Oct 27, 2018

Acceptance test results

$ make testacc TEST=./pagerduty/ TESTARGS='-run=TestAccPagerDutyService_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./pagerduty/ -v -run=TestAccPagerDutyService_* -timeout 120m
=== RUN   TestAccPagerDutyServiceIntegration_import
--- PASS: TestAccPagerDutyServiceIntegration_import (10.12s)
=== RUN   TestAccPagerDutyService_import
--- PASS: TestAccPagerDutyService_import (8.02s)
=== RUN   TestAccPagerDutyServiceWithIncidentUrgency_import
--- PASS: TestAccPagerDutyServiceWithIncidentUrgency_import (8.11s)
=== RUN   TestAccPagerDutyServiceIntegration_Basic
--- PASS: TestAccPagerDutyServiceIntegration_Basic (21.28s)
=== RUN   TestAccPagerDutyServiceIntegrationGeneric_Basic
--- PASS: TestAccPagerDutyServiceIntegrationGeneric_Basic (15.47s)
=== RUN   TestAccPagerDutyService_Basic
--- PASS: TestAccPagerDutyService_Basic (15.14s)
=== RUN   TestAccPagerDutyService_BasicWithIncidentUrgencyRules
--- PASS: TestAccPagerDutyService_BasicWithIncidentUrgencyRules (15.18s)
=== RUN   TestAccPagerDutyService_FromBasicToCustomIncidentUrgencyRules
--- PASS: TestAccPagerDutyService_FromBasicToCustomIncidentUrgencyRules (12.47s)
=== RUN   TestAccPagerDutyService_SupportHoursChange
--- PASS: TestAccPagerDutyService_SupportHoursChange (11.59s)
PASS
ok      github.com/terraform-providers/terraform-provider-pagerduty/pagerduty   117.485s

@sarkis
Copy link
Contributor Author

sarkis commented Oct 29, 2018

@heimweh I was debating where this should be fixed (in this provider here or in the api call in https://github.com/heimweh/go-pagerduty). I think this is the better of the two solutions, but am open to move this out to go-pagerduty if you think that would be a better place for this.

@sarkis
Copy link
Contributor Author

sarkis commented Nov 2, 2018

hi @heimweh anything else I should do here to have this PR ready to review?

@porkcharsui
Copy link

@sarkis Just tested out your branch, and it worked great, resolving the blocking bug I encountered (#97).

@heimweh Is there anything I can do to help expedite this landing to master?

Copy link
Collaborator

@heimweh heimweh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sarkis, sorry for the delay here. I've been away for a couple of days.

Thank you so much for this!

This LGTM 👍

@heimweh heimweh merged commit 891b712 into PagerDuty:master Nov 2, 2018
@sarkis
Copy link
Contributor Author

sarkis commented Nov 2, 2018

@heimweh no worries at all! I apologize for the spam... Thanks so much for this awesome provider. Hoping to help out with more open issues in the near future.

@sarkis
Copy link
Contributor Author

sarkis commented Nov 4, 2018

@heimweh is there some set release cadence? I'm managing a couple of PRs in our terraform manifests that will require the change from this PR, so I wanted to check in to see when 1.3 may be released, before I set an expectation here.

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

Successfully merging this pull request may close these issues.

Service using supported hours and severity based escalation - problem with scheduled actions
3 participants