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

feat: extract retry config from resources and added default retry config #735

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

hqhqhqhqhqhqhqhqhqhqhq
Copy link
Collaborator

This pr extract the repetitive code in setting up client with retry and added default retry configs when custom retry logics not provided.
Also solves #691

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 12 changed files in this pull request and generated no comments.

Files not reviewed (7)
  • internal/services/common.go: Evaluated as low risk
  • internal/services/azapi_resource_action_ephemeral.go: Evaluated as low risk
  • internal/services/azapi_resource_action_resource.go: Evaluated as low risk
  • internal/services/azapi_resource_list_data_source.go: Evaluated as low risk
  • internal/services/azapi_resource_action_data_source.go: Evaluated as low risk
  • internal/services/azapi_resource_data_source.go: Evaluated as low risk
  • internal/retry/retryable_errors.go: Evaluated as low risk
Comments suppressed due to low confidence (2)

internal/clients/data_plane_client.go:544

  • Ensure that the input to StringSliceToRegexpSliceMust is always valid to prevent panics from invalid regex patterns.
errRegExps = StringSliceToRegexpSliceMust(retry.GetErrorMessages())

internal/clients/data_plane_client.go:557

  • Add a test case to verify that RetryGet correctly handles the environment variable and falls back to the default value if the variable is not set or invalid.
return 2 * time.Minute
)

// Create a new retry client to handle specific case of transient 404 or empty body after resource creation or other retryable errors
clientGetAfterPut := r.ProviderData.ResourceClient.ConfigureClientWithCustomRetry(ctx, plan.Retry)
Copy link
Member

Choose a reason for hiding this comment

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

The retry for the get after put is a special case and does not use the configuration in the resource's retry attribute. Removing the above code will cause a regression for resources deployed at MG scope, as this relies on the response check function:

			func(d interface{}) bool {
				return d == nil
			},

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.

2 participants