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

Feature: Retry errors from CCI 3 times in order to sidestep retryable errors #72

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

Conversation

AlexanderMann
Copy link

Motivation

#70

^ seeing the same errors as above...when running this...from inside CCI. This is the simplest solution I could whip up. I'm not a Go native developer, but figured this would be a good enough first pass! Happy to try and make changes as suggested etc.

Thanks for making this awesome provider! Our ops team is really happy with it 😄

Suggested Musical Pairing

https://soundcloud.com/madonna/hung-up-album-version

Copy link
Collaborator

@bendrucker bendrucker left a comment

Choose a reason for hiding this comment

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

Thanks, https://github.com/hashicorp/go-retryablehttp would be a good fit for this. That will handle potentially transient server response errors in addition to client errors like connection failures. I'm not sure this code all requests:

The main difference from net/http is that requests which take a request body (POST/PUT et. al) can have the body provided in a number of ways (some more or less efficient) that allow "rewinding" the request body if the initial request fails so that the full request can be attempted again.

It also offers logging hooks which would seem like a useful thing to implement:

https://www.terraform.io/plugin/sdkv2/debugging#log-based-debugging

@bendrucker
Copy link
Collaborator

Also, from #70:

When I try to apply again the resources fails because it already exists.

This would suggest that retrying would not be the answer. A client is left in a hard spot here. It needs to fetch the context by ID. Reads can easily be retried, but if that write fails what can the client do?

In theory, yes, you can list and match by name, but that is not the provider's job. Terraform intentionally makes import an explicit operation.

Retrying is reasonable in general but I wonder if adding it just results in a new error for the original case described in #70, where upon retry (manual or automatic) Circle will return a 4xx, hopefully 409, indicating a conflict.

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