-
Notifications
You must be signed in to change notification settings - Fork 15
Add explicit context checking for belt-and-suspenders safety in tenant isolation policy polling #801
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
base: main
Are you sure you want to change the base?
Add explicit context checking for belt-and-suspenders safety in tenant isolation policy polling #801
Changes from 9 commits
b2c7560
77018bb
8934fc5
1d35110
38d3dfe
0b2b467
e69df34
da12283
0ee6ab3
13b6f88
20f5aeb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
kind: fixed | ||
body: Add explicit context checking for belt-and-suspenders safety in tenant isolation policy polling | ||
time: 2025-06-03T13:03:29.272364174Z | ||
custom: | ||
Issue: "800" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,7 +144,8 @@ func getRetryAfterDuration(resp *http.Response) time.Duration { | |
// doWaitForLifecycleOperationStatus polls an asynchronous operation until completion. | ||
// It follows the location header from the initial response, respects Retry-After | ||
// headers to control polling frequency, and implements exponential backoff with | ||
// minimum 2s and maximum 60s between attempts. | ||
// minimum 2s and maximum 60s between attempts. The function respects context | ||
// cancellation during sleep periods between polling attempts. | ||
// | ||
// Parameters: | ||
// - ctx: Context for the request with cancellation and timeout capabilities | ||
|
@@ -186,10 +187,17 @@ func (client *Client) doWaitForLifecycleOperationStatus(ctx context.Context, res | |
|
||
tflog.Debug(ctx, fmt.Sprintf("Waiting for %s before polling again", waitTime)) | ||
|
||
// Wait before polling again | ||
err = client.Api.SleepWithContext(ctx, waitTime) | ||
if err != nil { | ||
return nil, fmt.Errorf("polling interrupted: %w", err) | ||
// Check context before sleep for belt-and-suspenders safety | ||
select { | ||
case <-ctx.Done(): | ||
tflog.Debug(ctx, "Polling cancelled due to context cancellation") | ||
return nil, fmt.Errorf("polling cancelled: %w", ctx.Err()) | ||
mawasile marked this conversation as resolved.
Show resolved
Hide resolved
|
||
default: | ||
// Wait before polling again (honors context cancellation) | ||
err = client.Api.SleepWithContext(ctx, waitTime) | ||
if err != nil { | ||
return nil, fmt.Errorf("polling interrupted: %w", err) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this change is needed since SleepWithContext already checks for context cancellation. If this is needed in addition what sleepwithcontext already does then it should probably be added to sleepwithcontext and not only in this api. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're absolutely right. The explicit context check was redundant since |
||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.