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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port Output -versioned function invokes #163

Closed
1 task done
Tracked by #598
t0yv0 opened this issue Feb 17, 2022 · 4 comments
Closed
1 task done
Tracked by #598

Port Output -versioned function invokes #163

t0yv0 opened this issue Feb 17, 2022 · 4 comments
Assignees
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Milestone

Comments

@t0yv0
Copy link
Member

t0yv0 commented Feb 17, 2022

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Currently invoking a function accepts plain arguments and returns a CompleteableFuture. We instead want it to accept builders that overload T and Output<T> in the arguments, and returns an Output<Result> as the result.

For an example illustrating the design consider this snippet from azure template. It is now possible to pass Output values to invokes directly:

Design

    private static Output<String> getStorageAccountPrimaryKey(Output<String> resourceGroupName,
                                                              Output<String> accountName) {
       return StorageFunctions.listStorageAccountKeys(ListStorageAccountKeysArgs.builder()
                                                       .resourceGroupName(resourceGroupName)
                                                       .accountName(accountName)
                                                       .build())
            .applyValue(r -> r.keys().get(0).value())
            .asSecret();
    }

Before the change one had to manually await the Output args and project the resulting CompletableFuture to an Output.

    private static Output<String> getStorageAccountPrimaryKey(Output<String> resourceGroupName,
                                                              Output<String> accountName) {
        return Output.tuple(resourceGroupName, accountName).apply(tuple -> {
            var actualResourceGroupName = tuple.t1;
            var actualAccountName = tuple.t2;
            var invokeResult = StorageFunctions.listStorageAccountKeys(ListStorageAccountKeysArgs.builder()
                    .resourceGroupName(actualResourceGroupName)
                    .accountName(actualAccountName)
                    .build(), InvokeOptions.Empty);
            return Output.of(invokeResult)
                    .applyValue(r -> r.keys().get(0).value())
                    .asSecret();
        });

Breaking Changes

When Java SDKs for providers such as azure-native update to the version of codegen implementing this feature, it will be a breaking change. To upgrade your code, you have two options:

  1. rewrite the code to use the new form; note for example that StorageFunctions.listStorageAccountKeys returns an Output instead of a CompletableFuture, and ListStorageAccountKeysArgs now accepts Output values.

  2. rewrite the code that uses the old form; it is now renamed with a Plain suffix. So for our example, you could use StorageFunctions.listStorageAccountKeysPlain that returns a CompletableFuture as before.

Follow up

Affected area/feature

@t0yv0 t0yv0 added the kind/enhancement Improvements or new features label Feb 17, 2022
This was referenced Mar 31, 2022
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 8, 2022

Not in preview CC @mikhailshilkov

@t0yv0
Copy link
Member Author

t0yv0 commented Apr 8, 2022

TBD @dixler pre-plan how this looks like so plain Function calls don't grab names we want to use for Output versioned calls that we want to make "easier" or "more default".

@t0yv0 t0yv0 changed the title Port Output -versioned invokes Port Output -versioned function invokes Apr 26, 2022
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 26, 2022

NOTE: when doing this, also take care to update these functions that are used in docs generation:

func (d DocLanguageHelper) GetFunctionName(modName string, f *schema.Function) string {
	return tokenToFunctionName(f.Token)
}

func (d DocLanguageHelper) GetResourceFunctionResultName(modName string, f *schema.Function) string {
	return tokenToFunctionResultClassName(f.Token).String()
}

@dixler dixler self-assigned this Apr 26, 2022
@dixler
Copy link
Contributor

dixler commented Apr 26, 2022

stepping back from this. seems a little more involved and I have to put together a demo and apply can workaround this.

@dixler dixler removed their assignment Apr 26, 2022
@Zaid-Ajaj Zaid-Ajaj self-assigned this May 10, 2022
@mikhailshilkov mikhailshilkov added this to the 0.73 milestone May 17, 2022
@mikhailshilkov mikhailshilkov modified the milestones: 0.73, 0.74 Jun 7, 2022
@t0yv0 t0yv0 closed this as completed in #612 Jun 7, 2022
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

5 participants