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

Add retry config for PagerDuty/go-pagerduty #841

Merged

Conversation

imjaroiswebdev
Copy link
Contributor

Closes #824

Gracefully handle blocked API calls by 429 rate limit errors, when they are being issued by PagerDuty/go-pagerduty API client.

Acceptance tests results for Terraform objects utilizing PagerDuty/go-pagerduty client...

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccDataSourcePagerDutyBusinessService -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     (cached) [no tests to run]
=== RUN   TestAccDataSourcePagerDutyBusinessService_Basic
--- PASS: TestAccDataSourcePagerDutyBusinessService_Basic (13.13s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       13.588s
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  (cached) [no tests to run]
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccDataSourcePagerDutyStandardsResourceScores -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     (cached) [no tests to run]
=== RUN   TestAccDataSourcePagerDutyStandardsResourceScores_Basic
--- PASS: TestAccDataSourcePagerDutyStandardsResourceScores_Basic (24.08s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       24.586s
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  (cached) [no tests to run]
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccDataSourcePagerDutyStandardsResourcesScores -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     (cached) [no tests to run]
=== RUN   TestAccDataSourcePagerDutyStandardsResourcesScores_Basic
--- PASS: TestAccDataSourcePagerDutyStandardsResourcesScores_Basic (23.12s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       23.591s
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  (cached) [no tests to run]
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccDataSourcePagerDutyStandards -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     (cached) [no tests to run]
=== RUN   TestAccDataSourcePagerDutyStandardsResourceScores_Basic
--- PASS: TestAccDataSourcePagerDutyStandardsResourceScores_Basic (28.26s)
=== RUN   TestAccDataSourcePagerDutyStandardsResourcesScores_Basic
--- PASS: TestAccDataSourcePagerDutyStandardsResourcesScores_Basic (21.72s)
=== RUN   TestAccDataSourcePagerDutyStandards_Basic
--- PASS: TestAccDataSourcePagerDutyStandards_Basic (9.81s)
=== RUN   TestAccDataSourcePagerDutyStandards_WithResourceType
--- PASS: TestAccDataSourcePagerDutyStandards_WithResourceType (10.30s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       70.581s
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  (cached) [no tests to run]
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccPagerDutyBusinessService -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_import
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_import (21.07s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_User
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_User (19.33s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_Team
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_Team (17.71s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_TeamUser
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_TeamUser (17.77s)
=== RUN   TestAccPagerDutyBusinessServiceDependency_Basic
--- PASS: TestAccPagerDutyBusinessServiceDependency_Basic (45.89s)
=== RUN   TestAccPagerDutyBusinessServiceDependency_Parallel
--- PASS: TestAccPagerDutyBusinessServiceDependency_Parallel (60.18s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     183.149s
=== RUN   TestAccPagerDutyBusinessService_import
--- PASS: TestAccPagerDutyBusinessService_import (19.21s)
=== RUN   TestAccPagerDutyBusinessService_Basic
--- PASS: TestAccPagerDutyBusinessService_Basic (21.50s)
=== RUN   TestAccPagerDutyBusinessService_WithTeam
--- PASS: TestAccPagerDutyBusinessService_WithTeam (19.34s)
=== RUN   TestAccPagerDutyBusinessService_SDKv2Compatibility
--- PASS: TestAccPagerDutyBusinessService_SDKv2Compatibility (39.44s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       100.455s
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  0.595s [no tests to run]
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccPagerDutyBusinessService -timeout 120m
?       github.com/PagerDuty/terraform-provider-pagerduty       [no test files]
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_import
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_import (21.07s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_User
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_User (19.33s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_Team
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_Team (17.71s)
=== RUN   TestAccPagerDutyBusinessServiceSubscriber_TeamUser
--- PASS: TestAccPagerDutyBusinessServiceSubscriber_TeamUser (17.77s)
=== RUN   TestAccPagerDutyBusinessServiceDependency_Basic
--- PASS: TestAccPagerDutyBusinessServiceDependency_Basic (45.89s)
=== RUN   TestAccPagerDutyBusinessServiceDependency_Parallel
--- PASS: TestAccPagerDutyBusinessServiceDependency_Parallel (60.18s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerduty     (cached)
=== RUN   TestAccPagerDutyBusinessService_import
--- PASS: TestAccPagerDutyBusinessService_import (19.21s)
=== RUN   TestAccPagerDutyBusinessService_Basic
--- PASS: TestAccPagerDutyBusinessService_Basic (21.50s)
=== RUN   TestAccPagerDutyBusinessService_WithTeam
--- PASS: TestAccPagerDutyBusinessService_WithTeam (19.34s)
=== RUN   TestAccPagerDutyBusinessService_SDKv2Compatibility
--- PASS: TestAccPagerDutyBusinessService_SDKv2Compatibility (39.44s)
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/pagerdutyplugin       (cached)
testing: warning: no tests to run
PASS
ok      github.com/PagerDuty/terraform-provider-pagerduty/util  (cached) [no tests to run]

@imjaroiswebdev imjaroiswebdev merged commit 6d9d656 into PagerDuty:master Apr 3, 2024
1 check passed
@@ -80,10 +80,14 @@ func (c *Config) Client(ctx context.Context) (*pagerduty.Client, error) {
apiUrl = c.ApiUrlOverride
}

maxRetries := 1

Choose a reason for hiding this comment

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

@imjaroiswebdev I have a feeling that a single retry might not be enough for larger configurations in terraform. Terraform is running 10 operations in parallel by default, so we could still fail when API responses are fast and we're unlucky with the timing. My gut feeling would set this to 3 retries (for HTTP 429) and maybe with a smaller retryInterval

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

Successfully merging this pull request may close these issues.

HTTP 429 Rate limit is no longer handled gracefully
2 participants