Fix custom timeout propagation for Update #1749
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1749 +/- ##
==========================================
- Coverage 59.75% 59.26% -0.50%
==========================================
Files 300 307 +7
Lines 42025 42403 +378
==========================================
+ Hits 25111 25129 +18
- Misses 15488 15852 +364
+ Partials 1426 1422 -4 ☔ View full report in Codecov by Sentry. |
| } | ||
| switch tc.cud { | ||
| case "Update", "Delete": | ||
| return "TODO[pulumi/pulumi-terraform-bridge#1651] - schema-specified timeouts are not " + |
There was a problem hiding this comment.
This suspicion is now confirmed for #1651 - custom Delete or Update timeouts from the upstream provider schema are not currently respected. Any chance that's by design? If we want it respected I can follow up with a quick PR here.
There was a problem hiding this comment.
Unless we can find a reason why they are not respected, let's respect them.
This is less Chesterton's fence then a pile of scattered sticks in a field.
| @@ -1684,7 +1684,7 @@ func transformFromState( | |||
| func newTimeoutOverrides(key shim.TimeoutKey, maybeTimeoutSeconds float64) map[shim.TimeoutKey]time.Duration { | |||
| timeoutOverrides := map[shim.TimeoutKey]time.Duration{} | |||
| if maybeTimeoutSeconds != 0 { | |||
| timeoutOverrides[shim.TimeoutCreate] = time.Duration(maybeTimeoutSeconds * float64(time.Second)) | |||
| timeoutOverrides[key] = time.Duration(maybeTimeoutSeconds * float64(time.Second)) | |||
There was a problem hiding this comment.
Shouldn't we have a lint for unused function args? That would have caught this.
There was a problem hiding this comment.
😢
pulumi-terraform-bridge/.golangci.yml
Lines 24 to 26 in 22fdb29
| } | ||
| switch tc.cud { | ||
| case "Update", "Delete": | ||
| return "TODO[pulumi/pulumi-terraform-bridge#1651] - schema-specified timeouts are not " + |
There was a problem hiding this comment.
Unless we can find a reason why they are not respected, let's respect them.
This is less Chesterton's fence then a pile of scattered sticks in a field.
Fixes a regression responsible for the pulumi/pulumi-aws#3442 P1.
When refactoring timeout handling in #1648 the behavior of custom timeouts for Update and Delete was inadvertently broken, but passed the test suite.
The regression was introduced in v3.72.0 of the bridge and affects all SDKv2 based resources. Custom timeouts were not respected for Update and Delete but were respected for Create.
The fix is very self-contained but this PR also backfills test coverage with a matrix of tests for custom timeout propagation and respecting both schema-based and user-based timeouts.