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.

Sign up for GitHub

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

Jump to bottom

Add retry config for PagerDuty/go-pagerduty #841

Merged
merged 1 commit into from
Apr 3, 2024
Merged

Add retry config for PagerDuty/go-pagerduty #841

merged 1 commit into from
Apr 3, 2024

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
mflis-sumo
mflis-sumo reviewed Apr 4, 2024
View reviewed changes
pagerdutyplugin/config.go
@@ -80,10 +80,14 @@ func (c *Config) Client(ctx context.Context) (*pagerduty.Client, error) {
apiUrl = c.ApiUrlOverride
}

maxRetries := 1
Copy link

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
Reviewers

@mflis-sumo mflis-sumo mflis-sumo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HTTP 429 Rate limit is no longer handled gracefully
2 participants
@imjaroiswebdev @mflis-sumo