Skip to content
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

Add support for deserializing output values #298

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Conversation

justinvp
Copy link
Member

This change adds support for deserializing output values and ensures they work for transforms. This is necessary as the engine will send output values to transforms, so we need to handle deserializing them before calling the transform and then properly serializing them when returning the results.

Fixes #297

@justinvp justinvp requested a review from a team as a code owner July 17, 2024 02:44
This change adds support for deserializing output values for transforms. This is necessary as the engine will send output values to transforms, so we need to handle deserializing them before calling the transform and then properly serializing them when returning the results.
justinvp added a commit to pulumi/docs that referenced this pull request Jul 17, 2024
@justinvp justinvp mentioned this pull request Jul 17, 2024
4 tasks
// The data from the engine will include output values, which will be deserialized
// into instances of `Output<T>`, which will track whether the value is known,
// is secret, and dependencies. We can ignore those values on outputData.
props.Add(kv.Key, outputData.Value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this still need to check the OutputData for known/secretness? Doesn't look like Deserialise is returning Output for those, just for actual output wire objects.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. Shouldn't everything be coming across from the engine to transforms as output values? In which case, those will already have the known/secretness/dependency tracking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For transforms yes, but isn't this used for other deserialisation for invokes/resources?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no ignore me, this is specific to transforms. Should be fine then.

@Frassle Frassle added this pull request to the merge queue Jul 18, 2024
Merged via the queue into main with commit 2a07ad6 Jul 18, 2024
17 checks passed
@Frassle Frassle deleted the justin/outputvalues branch July 18, 2024 15:48
@pulumi-bot
Copy link

This PR has been shipped in release v3.65.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transforms aren't deserializing output values
3 participants