Respect schema timeouts on Delete and Update #1751
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1751 +/- ##
==========================================
- Coverage 59.79% 59.28% -0.51%
==========================================
Files 300 307 +7
Lines 42025 42410 +385
==========================================
+ Hits 25129 25143 +14
- Misses 15474 15843 +369
- Partials 1422 1424 +2 ☔ View full report in Codecov by Sentry. |
| @@ -1350,6 +1357,7 @@ func (p *Provider) Delete(ctx context.Context, req *pulumirpc.DeleteRequest) (*p | |||
| // Create a new destroy diff. | |||
| diff := p.tf.NewDestroyDiff(ctx, res.TFName, shim.TimeoutOptions{ | |||
| TimeoutOverrides: newTimeoutOverrides(shim.TimeoutDelete, req.Timeout), | |||
| ResourceTimeout: res.TF.Timeouts(), | |||
There was a problem hiding this comment.
Why do you not need to decode them here but had to decode the timeouts in Update?
There was a problem hiding this comment.
Yeah it looks really weird. I copied what's in Create to what's in Update however in Delete there's no data to decode. I'm wondering if these two forms are equivalent actually.
There was a problem hiding this comment.
I can take a deeper dive to figure out if we can just use res.TF.Timeouts() everywhere.
There was a problem hiding this comment.
#1753 I've posted findings here, there is a slight difference and I'm inclined to keep it as is with keeping the mystery for later.
There was a problem hiding this comment.
I'm happy with that, just wanted to make sure it is intentional
Fixes #1651
With this change, resources written against SDKv2 that specify a preferred timeout for Update and Delete operations in their schema will now have this preference respected in the bridged providers.