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

Support Spaces Key API #1309

Closed
wants to merge 3 commits into from

Conversation

lee-aaron
Copy link
Contributor

Adds support for Spaces Key API based on digitalocean/openapi#958

@lee-aaron lee-aaron force-pushed the support-Spaces-API branch 2 times, most recently from 8c45dad to bb5af54 Compare January 22, 2025 20:01
@lee-aaron lee-aaron force-pushed the support-Spaces-API branch 8 times, most recently from 186198b to ad1edb3 Compare January 23, 2025 17:08
log.Printf("[DEBUG] Creating new Spaces key")
key, _, err = client.SpacesKeys.Create(ctx, req)
if err != nil {
return retry.RetryableError(
Copy link
Member

Choose a reason for hiding this comment

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

Doing some testing, I realized that that this was retrying a 403 Forbidden repeatedly. You'll probably want to inspect this error before returning a RetryableError.

By default, the godo client is configured to retry 500-level errors. So retry.RetryContext is generally only needed if there are specific cases you are trying to work around.

For example:

// Moving resources is async and projects can not be deleted till empty. Retries may be required.
err := retry.RetryContext(ctx, d.Timeout(schema.TimeoutDelete), func() *retry.RetryError {
_, err := client.Projects.Delete(context.Background(), projectID)
if err != nil {
if util.IsDigitalOceanError(err, http.StatusPreconditionFailed, "cannot delete a project with resources") {
log.Printf("[DEBUG] Received %s, retrying project deletion", err.Error())
return retry.RetryableError(err)
}
return retry.NonRetryableError(err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put in a few places where retries shouldn't happen

Copy link
Member

Choose a reason for hiding this comment

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

What scenario are you thinking that we'd need to retry a create request for? For example, sometimes we have to retry a request if we know we have to wait for another resource to be in a ready state (waiting on new database to accept connections, or a new droplet to be accessible etc.) or if we want a resource to be a in a certain state before we can use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

None of the bucket keys related scenarios are async in that sense. We don't have to wait for anything to be in a specific state.

Copy link
Member

Choose a reason for hiding this comment

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

Ok sweet, we're good to remove it then.

@lee-aaron lee-aaron force-pushed the support-Spaces-API branch 2 times, most recently from 279d6d4 to e7b469c Compare January 23, 2025 19:20
@andrewsomething andrewsomething self-assigned this Jan 28, 2025
@lee-aaron lee-aaron force-pushed the support-Spaces-API branch 4 times, most recently from cc7d2d9 to 8e33402 Compare February 20, 2025 17:30
@lee-aaron lee-aaron force-pushed the support-Spaces-API branch from 9b7df31 to 0e55d8a Compare March 11, 2025 16:17
@lee-aaron lee-aaron force-pushed the support-Spaces-API branch from 0e55d8a to a2aca42 Compare March 13, 2025 17:20
@lee-aaron lee-aaron force-pushed the support-Spaces-API branch from e8d46c4 to 38e1ea8 Compare March 13, 2025 17:42
Comment on lines +157 to +171
err := retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError {
log.Printf("[DEBUG] Deleting Spaces key: %s", d.Id())
_, err := client.SpacesKeys.Delete(ctx, d.Id())
if err != nil {
if util.IsDigitalOceanError(err, http.StatusInternalServerError, "") || util.IsDigitalOceanError(err, http.StatusPreconditionFailed, "") {
return retry.NonRetryableError(err)
}
return retry.RetryableError(
fmt.Errorf("[WARN] Error deleting Spaces key: %s, retrying: %s", d.Id(), err))
}
return nil
})
if err != nil {
return diag.Errorf("Error deleting Spaces key: %s", err)
}
Copy link
Contributor

@amahajan-do amahajan-do Mar 19, 2025

Choose a reason for hiding this comment

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

Suggested change
err := retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError {
log.Printf("[DEBUG] Deleting Spaces key: %s", d.Id())
_, err := client.SpacesKeys.Delete(ctx, d.Id())
if err != nil {
if util.IsDigitalOceanError(err, http.StatusInternalServerError, "") || util.IsDigitalOceanError(err, http.StatusPreconditionFailed, "") {
return retry.NonRetryableError(err)
}
return retry.RetryableError(
fmt.Errorf("[WARN] Error deleting Spaces key: %s, retrying: %s", d.Id(), err))
}
return nil
})
if err != nil {
return diag.Errorf("Error deleting Spaces key: %s", err)
}
log.Printf("[DEBUG] Deleting Spaces key: %s", d.Id())
_, err := client.SpacesKeys.Delete(ctx, d.Id())
if err != nil {
return diag.Errorf("Error deleting Spaces key: %s", err)
}

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.

4 participants