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

[sdk/nodejs] Keep prompt values prompt in construct #6522

Merged
merged 3 commits into from
Apr 9, 2021

Conversation

justinvp
Copy link
Member

@justinvp justinvp commented Mar 12, 2021

In order to support prompt values in multi-lang components, if an input value is prompt, keep it as-is instead of wrapping it in an Output.

Part of #6425

In order to support prompt values in multi-lang components, if an input value is prompt, keep it as-is instead of wrapping it in an Output.
inputs[k] = new Output(deps, Promise.resolve(runtime.unwrapRpcSecret(input)), Promise.resolve(true),
Promise.resolve(runtime.isRpcSecret(input)), Promise.resolve([]));
const isSecret = runtime.isRpcSecret(input);
if (!isSecret && deps.length === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This made me think a little bit, trying to type check this in my head. Somehow from the test it's evident that children: Optional[Int] travels through this code just fine, but how is that value represented here is not obvious to me. I also wonder about the condition (!isSecret && deps.length === 0). I don't know yet what inputDependencies are very well.

Would this code confuse T and Input<T> or it does not matter? I imagine there is a way to lift pure :: T -> Input<T> and that pure(x) should not have any dependencies so presumably it would satisfy deps.length == 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

inputDependencies is documented in the .proto:

map<string, PropertyDependencies> inputDependencies = 11; // a map from property keys to the dependencies of the property.

// PropertyDependencies describes the resources that a particular property depends on.
message PropertyDependencies {
repeated string urns = 1; // A list of URNs this property depends on.
}

If the property was assigned an output of another resource, or the result of an apply from one or more outputs, then the property will have an associated list of resource URNs that the property depends on. If there aren't any dependencies associated with the property, then we don't need to project it as an Output because we don't have any dependencies to track. Unless it's a secret: secrets in the type system are typed as Output<T> (we did this rather than having a separate Secret<T> type to keep the programming model simpler), so in that case, we need to wrap the value as Output<T>.

We don't have a specific flag that says "Property 'foo' was set a plain value", so instead, we just see whether the raw gRPC value can remain a plain value, otherwise we wrap it as an output. We're sort of relying on the type system of the generated SDK to indicate that the type of the value should be T and not Input<T>, but in some languages that don't have a real type system (e.g. JavaScript/Python), someone could force passing an Output, so right now we're sort of relying on type checking to prevent that. We should probably add some runtime type checking for JS/Python for plain values, to further help prevent that from happening.

Would this code confuse T and Input<T> or it does not matter?

Input<T> can be T or Output<T>:

export type Input<T> = T | Promise<T> | OutputInstance<T>;

If an input property is typed as accepting an Input<T>, it can be given a T, and it'll just work.

t.Fatalf("failed to build test component PATH: %v", err)
}

// TODO[pulumi/pulumi#5455]: Dynamic providers fail to load when used from multi-lang components.
Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize there's a problem here want to take a look now.

@justinvp justinvp merged commit f7cc19f into master Apr 9, 2021
@pulumi-bot pulumi-bot deleted the justin/plain_node_test branch April 9, 2021 21:36
abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
In order to support prompt values in multi-lang components, if an input value is prompt, keep it as-is instead of wrapping it in an Output.
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.

None yet

3 participants