core: get a cancelation context to the ephemeral resources #35778
Conversation
The existing Stopped method predates the use of a context, returning a simple channel for signaling cancellation. The method was not used however, so we can just update it to return a context to give us access to cancellation for ephemeral values.
jbardin
force-pushed
the
jbardin/ephemeral-cancel
branch
from
64a9f41
to
cd2631d
Compare
| // We might be closing due to a context cancellation, but we still need to | ||
| // be able to make non-canceled Close requests. | ||
| // | ||
| // TODO: if we're going to ignore the cancellation to ensure that Close is |
There was a problem hiding this comment.
I think a timeout (or a force cancel?) would make sense
There was a problem hiding this comment.
I'll have to think on that, since anytime you pick a timeout it's going to be wrong for some situation. This is already in a path that shouldn't be happening under normal circumstances though, so maybe it doesn't really matter so much.
|
We might be able to hook tracing into that, but the context handling in Terraform is a bit strange due to it's CLI nature. We also need to decide exactly what we'd want to get from tracing that logging doesn't already give us, the graph evaluation tends to be the uninteresting part when tracking down runtime issues. |
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This turned out to be a very simple change to get the correct cancellation context into ephemeral resources. The internal terraform
EvalContexthas a very oldStoppedmethod which returned a channel instead of aContext, and was never used.Replace the
Stoppedmethod withStopCtxwhich returns the fullcontext.Contextso we can forward that into the ephemeral resource calls.