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

Functions/datasources should accept Outputs #5758

Closed
18 of 19 tasks
lukehoban opened this issue Nov 13, 2020 · 15 comments · Fixed by #7825
Closed
18 of 19 tasks

Functions/datasources should accept Outputs #5758

lukehoban opened this issue Nov 13, 2020 · 15 comments · Fixed by #7825
Assignees
Labels
impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/L Estimated effort to complete (up to 10 days).
Milestone

Comments

@lukehoban
Copy link
Member

lukehoban commented Nov 13, 2020

One of the most common usability issues I've seen new users hit with Pulumi is that datasources/functions take and return raw values instead of Inputs/Outputs, meaning they are very difficult to compose with the rest of the Pulumi resource model. We encourage folks to get used to writing code like this in the normal Pulumi usage:

const a = new ResourceA("name");
const b = new ResourceB("name", { prop: a.id }); // pass an `Output` as an `Input`

But then when you want to do the same with a Pulumi function - it doesn't work.

const a = new ResourceA("name");
const b = someFunction({ prop: a.id }); // cannot pass `Output` where `string` is expected

Instead - you have to write something like this:

const a = new ResourceA("name");
const b = a.id.apply(v => someFunction({ prop: v }));

This is not obvious, and not pleasant.

The original motivation was to provide the additional flexibility of being able to use these functions in cases where you don't need to get caught up in Outputs. And it may be we feel we have to continue to support those use cases.

A non-breaking way to improve this would be to project two copies of every function somFunc and someFuncOutput for example - where the latter accepts Inputs and returns Outputs.


Engineering Work Items

M64

M63

M62

M61

Future Iterations

@lukehoban lukehoban added needs-triage Needs attention from the triage team kind/enhancement Improvements or new features impact/usability Something that impacts users' ability to use the product easily and intuitively labels Nov 13, 2020
@justinvp
Copy link
Member

Semi-related to #5385. For multi-lang components, we want to be able to support functions referencing resources. To do so, if a function takes a resource as an argument, we'll update codegen to emit the function as accepting Input<TheResource> along with any other args as inputs, and have the function return an Output.

@mikhailshilkov
Copy link
Member

Related to both comments above, I was playing with an idea of yet another form

const a = new ResourceA("name");
const b = a.someFunction();

This is based on Azure NextGen where invokes ~always "belong" to a parent resource and the relationship is easy to derive. WDYT?

@justinvp
Copy link
Member

const b = a.someFunction();

We are planning to add support for "resource methods" as part of the multi-lang component work. I send you a link to the design doc.

@pgavlin
Copy link
Member

pgavlin commented Nov 17, 2020

A non-breaking way to improve this would be to project two copies of every function somFunc and someFuncOutput for example - where the latter accepts Inputs and returns Outputs.

I'm not sure that I agree that this is an improvement, especially for the first-time experience. How do we describe when to use which? Do we update examples to use *Output? We have apply for a reason; let's figure out how to explain that better rather than muddying our API surface area.

@lukehoban
Copy link
Member Author

How do we describe when to use which?

~all documentation would point at the *Output form.

Do we update examples to use *Output?

Yes - almost all examples, and all code-generated examples, would use the *Output form - which will almost universally make the example code much cleaner as today our standard codegen pattern is to wrap these in apply anyway because we have to work around the lack of theses functions.

let's figure out how to explain that better rather than muddying our API surface area.

Having tried to explain this hundreds of times, at this point I believe we have to make a product change. Open to suggestions on alternative solutions, but this is just too significant of a usability issue to leave to just "explaining".

@pgavlin
Copy link
Member

pgavlin commented Nov 17, 2020

Do you believe that it's most common to call functions using outputs from resources and then use them as pure inputs to other resources (as opposed to using the results for control flow)?

@mikhailshilkov
Copy link
Member

Do you believe that it's most common to call functions using outputs from resources and then use them as pure inputs to other resources (as opposed to using the results for control flow)?

FWIW, this is true for my experience with Azure NextGen and beyond.

@lukehoban
Copy link
Member Author

As a concrete example - this is the code we will be adding to our azure Python template:

# Export the primary key of the Storage Account
primary_key = pulumi.Output.all(resource_group.name, account.name) \
    .apply(lambda args: storage.list_storage_account_keys(
        resource_group_name=args[0],
        account_name=args[1]
    )).apply(lambda accountKeys: accountKeys.keys[0].value)

If we had an Output form of the function - that whole thing would be:

# Export the primary key of the Storage Account
primary_key =  
storage.list_storage_account_keys_output(resource_group_name=resource_group.name, account_name=account.name).keys[0].value

@t0yv0
Copy link
Member

t0yv0 commented May 4, 2021

Tangentially related, this style of composition is what Haskellers eat for breakfast under the name of Applicative functors.

 pure    :: a -> f a
 ( <*> ) :: f (a -> b) -> f a -> f b

 liftA2 f x y = f <$> x <*> y
    
 pure storage.list_storage_account_keys <*> resource_group.name <*> account.name  ==
      storage.list_storage_account_keys <$> resource_group.name <*> account.name  ==
      liftA2 storage.list_storage_account_keys resource_group.name account.name

This version of lift_a2 checks under mypy:

from pulumi import Output
from typing import Optional, Callable, TypeVar, List, Any
import pulumi


class InvokeOptions:
    pass

class Key:
    value: str

class ListStorageAccountKeysResult:
    keys: List[Key]


def list_storage_account_keys(account_name: Optional[str] = None,
                              expand: Optional[str] = None,
                              resource_group_name: Optional[str] = None,
                              opts: Optional[InvokeOptions] = None) -> ListStorageAccountKeysResult:
    ...


T1 = TypeVar('T1')
T2 = TypeVar('T2')
T3 = TypeVar('T3')


def lift_a2(f: Callable[[T1, T2], T3]) -> Callable[[Output[T1], Output[T2]], Output[T3]]:

    def f_any(xy: Any) -> T3:
        return f(xy[0], xy[1])

    return lambda x, y: pulumi.Output.all([x, y]).apply(f_any)


def example(account_name: Output[Optional[str]], resource_group_name: Output[Optional[str]]) -> Output[str]:
    return lift_a2(list_storage_account_keys)(account_name, resource_group_name) \
        .apply(lambda x: x.keys[0].value)

@t0yv0
Copy link
Member

t0yv0 commented May 4, 2021

There is also a code style for this more suitable to imperative languages, I think I first saw it in Knockout JS library. The idea is that there is a magic "Output builder" function that accepts a pure (this is crucial) lambda that is allowed to evaluate any Output[T] to a T inside the body. This presents no problem for mypy checking again:

class Evaluator:
    def __call__(self, x: Output[T1]) -> T1:
        ...


def output(f: Callable[[Evaluator], T1]) -> Output[T1]:
    ...


def example2(account_name: Output[Optional[str]], resource_group_name: Output[Optional[str]]) -> Output[str]:
    return output(lambda e: list_storage_account_keys(e(account_name), e(resource_group_name)).keys[0].value)

The output function can be made general enough to support functor, applicative functor, and monadic composition. Applicative functor was just demonstrated. Functor is simple:


def a_to_b(x: A) -> B:
    ...

def functor_example(x: Output[A]) -> Output[B]:
    return output(lambda e: a_to_b(e(x)))

For monadic composition we need a variation that lets the body return a nested Output[T] and unwraps it later:

def output(f: Callable[[Evaluator], Output[T1]]) -> Output[T1]:
    ...

Purity of the builder lambda is important because the implementation needs to call the lambda at least twice, once with junk arguments, to introspect its body. The constraint is more than purity, in fact, it is assumed that the lambda does not introspect and branch on the arguments. This makes it slightly controversial. The notation is certainly super convenient but these restrictions may fly in the way of the programmer intuition. Again Knockout JS lib got away with this.

@t0yv0
Copy link
Member

t0yv0 commented May 4, 2021

Chatting with @pgavlin (thanks a ton), concrete steps for me here are:

  • table imperative output proposal into a separate issue for later discussion (low priority)

  • lift_a2 style combinators allowed as an implementation detail but not public API (no teaching the world Haskell)

  • provide a PR to edit SDK codegen in Pulumi to add the new form of functions

  • spilt out a ticket and follow up with a PR to update hcl2 codegen to take advantage of the new form in the code generated out of documentation - this might be a little more involved

@lukehoban
Copy link
Member Author

BTW - a sketch of what I expected this to look like for TypeScript is in 54dad85.

@leezen leezen removed this from the 0.56 milestone May 17, 2021
@t0yv0
Copy link
Member

t0yv0 commented Sep 10, 2021

Quick update, I'm sorry for some delay getting all the subtasks done here. After chatting with @emiliza I've broken down the remaining work so it's easier to see in our planning so we see where we stand on this.

Go and Python are done.

Remainder:

C# - PR done with tests but waiting on providers to up the SDK

Node

Update conceptual docs

Update docs gen so that this shows up in API reference documentation

Update examples to use this style

Update program gen so that generated examples use this style

Looks like I've already opened a set of similar issues before, let me clean this up as dups right now.

@ericrudder the list above may help answer your questions on the "big picture view" here, if not please let me know if you have further questions.

@emiliza emiliza modified the milestones: 0.61, 0.62 Sep 14, 2021
This was referenced Sep 15, 2021
@emiliza emiliza modified the milestones: 0.62, 0.63 Oct 1, 2021
@emiliza emiliza modified the milestones: 0.63, 0.64 Oct 25, 2021
@infin8x infin8x removed the resolution/fixed This issue was fixed label Nov 1, 2021
@emiliza emiliza modified the milestones: 0.64, 0.65 Nov 16, 2021
@t0yv0 t0yv0 closed this as completed Nov 18, 2021
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Nov 18, 2021
@lukehoban lukehoban changed the title Functions/datasources don't compose well with Pulumi model - should accept Outputs Functions/datasources should accept Outputs Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/L Estimated effort to complete (up to 10 days).
Projects
Status: 🚀 Shipped
9 participants