TPT-3454: Refactor wait functions to use context for timeouts and polling#964
Conversation
c9c70a9 to
d6dbf7d
Compare
d6dbf7d to
fdfa88a
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the SDK's WaitFor* family in waitfor.go so timeouts are driven entirely by the caller's context.Context instead of an explicit timeoutSeconds int parameter, and consolidates per-function poll loops into a generic poll[T] helper. Integration and unit tests are updated to construct contexts via a new waitContext(t, timeout) helper.
Changes:
- Removes
timeoutSecondsfrom many exportedWaitFor*functions and fromLKEClusterPollOptions, replacing the inline ticker/select loops with a shared genericpollhelper andnewTicker(client). - Adds a
waitContexthelper (inintegration_suite_test.go) plus a new unit test filetest/unit/waitfor_test.gocovering deadline, success, nil-LinodeID, and event-poller paths. - Threads
ctx context.ContextthroughsetupPostgresDatabase,setupMySQLDatabase,setupInstanceBackup,setupVolumeAttachedToLinode, andwaitForDatabaseUpdated, and updates all callers to pass the new context-derived timeouts.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| waitfor.go | Drops timeoutSeconds params, introduces generic poll/newTicker, refactors most WaitFor* funcs, adds previousEvents init in NewEventPoller. |
| test/integration/integration_suite_test.go | Adds waitContext(t, timeout) helper based on t.Context(). |
| test/unit/waitfor_test.go | New unit tests exercising context-driven wait behavior with httpmock. |
| test/integration/iam_io_ready_test.go | Threads ctx through setupVolumeAttachedToLinode; uses fresh ctx in cleanup. |
| test/integration/instance_snapshots_test.go | Adds ctx param to setupInstanceBackup, updates wait calls. |
| test/integration/instances_test.go | Adds per-test waitContext and updates many WaitFor* call sites. |
| test/integration/instance_config_test.go | Adds ctx for WaitForVolumeStatus calls. |
| test/integration/images_test.go | Adds ctx for WaitForImageStatus calls. |
| test/integration/image_sharegroups_test.go | Adds ctx for WaitForImageStatus. |
| test/integration/lke_clusters_test.go | Drops TimeoutSeconds, passes ctx to WaitForLKEClusterReady/WaitForLKEClusterStatus. |
| test/integration/lke_node_pools_test.go | Same ctx-based update for LKE waits. |
| test/integration/monitor_alert_definitions_test.go | Updates WaitForAlertDefinitionStatus call. |
| test/integration/monitor_api_service_test.go | Threads ctx through setup. |
| test/integration/monitor_services_token_creation_test.go | Passes ctx into setupPostgresDatabase. |
| test/integration/mysql_test.go / mysql_db_config_test.go | Adds ctx param to setupMySQLDatabase, updates DB waits. |
| test/integration/postgres_test.go / postgres_db_config_test.go | Adds ctx param to setupPostgresDatabase, updates DB waits. |
| test/integration/databases_test.go | Adds ctx param to waitForDatabaseUpdated. |
| test/integration/vlans_test.go | Adds ctx for WaitForInstanceStatus calls. |
| test/integration/volumes_test.go | Adds ctx for WaitForVolumeStatus/WaitForVolumeLinodeID calls. |
| test/integration/waitfor_test.go | Adds ctx for WaitForFinished/WaitForResourceFree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
67adcac to
db101f5
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ezilber-akamai
left a comment
There was a problem hiding this comment.
Changes look good and tests pass locally!
lgarber-akamai
left a comment
There was a problem hiding this comment.
Looks great and works well in my local testing, nice work!
* TPT-4423: Drop resty as a dependency (#938) * Ported resty removal changes to new branch from up-to-date main and made additional changes to monitor client * Fixed lint * Updated log formatting to more closely resemble Resty's * Fixed lint * redact request header log * Fixes * Fixed CoPilot suggestions * Sanitize log output * Addressed CoPilot comments --------- Co-authored-by: Zhiwei Liang <zliang@akamai.com> * TPT-4421: Remove deprecated fields and methods (#946) * Removed deprecated fields and methods and updated tests * Fix lint * Addressed copilot suggestion * Fixed lint * Removed V2 suffix from various fields and methods * Addressed CoPilot suggestions * Added separate struct for Instance IP Address update options * Address PR suggestions * TPT-4414: Refactor JSON tags to use "omitzero" for optional fields across multiple files (#947) * Refactor JSON struct tags to use "omitzero" for optional fields * Remove url tags in pagination structs * TPT-4414: Change slice pointer types with `omitzero` to be slice (#954) * Refactor: Change pointer slices to slices for various fields with omitzero * Refresh fixtures * Update TestLKECluster_Update to remove pinned lke versions in test * TPT-4434: Added custom type for Region Capabilities and updated tests (#953) * Added custom type for Region Capabilities and updated tests * Fix formatting * Added disctinction between Object Storage Bucket Access POST/PUT endpoints (#951) * TPT-3454: Refactor wait functions to use context for timeouts and polling (#964) * Refactor wait functions to use context for timeouts and polling * fix context cancelled in cleanup * nolint generic return type * Fix grammar Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Make preTask a private function; cleanup unnecessary previousEvents allocation * Fix error msg format Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * TPT-4422: Added options structs for POST/PUT endpoints that were missing them (#968) * Added options structs for POST/PUT endpoints that were missing them * Address PR comments * Address PR comments * TPT-3483: Update NewClient method to return an error rather than just logging errors (#967) * refactor: update NewClient and related functions to handle errors properly * refactor: update SetRootCertificate to return errors and enhance logging * update NewClient instantiation to handle errors * Export PaginatedResponse type in request_helpers (#970) * TPT-3388: Separated GET and POST/PUT options structs where needed (#969) * Separated GET and POST/PUT options structs where needed * Separated additional structs that were reused for options and response * More struct fixes * Fix lint * Rename FirewallRule structs * integration test fixes * Addressed PR comments * FirewallRuleSetRule options; test updates; fixture refreshed * Address more PR comments --------- Co-authored-by: Zhiwei Liang <zliang@akamai.com> * TPT-4464: Update FirewallDeviceEntity Label field to be a pointer (#971) * Update FirewallDeviceEntity Label field to be a pointer and adjust tests accordingly * Fix unit tests * fmt * Fix another unit test * Change `[]*T` to `[]T` in all structs (#972) * Remove LKE cluster dashboard function, tests, and related fixture (#975) * Simplify client.go (#976) Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me> * Apply suggestion from @zliang-akamai * Update region capability in TestImageSharing_Suite to use Linodes --------- Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me> Co-authored-by: Erik Zilber <ezilber@akamai.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Zhiwei Liang <zhiwei.liang@zliang.me>
📝 Description
Require timeout to be passed in context for the
WaitFor...functions and intentionally removetimeoutSecondsfrom functions' parameters.✔️ How to Test