Skip to content

Commit

Permalink
Merge #11596
Browse files Browse the repository at this point in the history
11596: [sdk/gen] Avoid generating Result types for functions with empty outputs r=Zaid-Ajaj a=Zaid-Ajaj

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Part of #11418 where we shouldn't generate result types for function invokes that have no properties. 

This is a separate PR to showcase the exact changes required to improve the situation and reduce the diff from #11418

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Zaid Ajaj <zaid.naom@gmail.com>
  • Loading branch information
bors[bot] and Zaid-Ajaj committed Dec 9, 2022
2 parents a0f2c1c + b0b4add commit c11a8eb
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 186 deletions.
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: sdkgen/dotnet,go,nodejs,python
description: Do not generate Result types for functions with empty outputs
4 changes: 2 additions & 2 deletions pkg/codegen/dotnet/gen.go
Expand Up @@ -1349,7 +1349,7 @@ func (mod *modContext) genFunction(w io.Writer, fun *schema.Function) error {
fmt.Fprintf(w, "{\n")

var typeParameter string
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
typeParameter = fmt.Sprintf("<%sResult>", className)
}

Expand Down Expand Up @@ -1412,7 +1412,7 @@ func (mod *modContext) genFunction(w io.Writer, fun *schema.Function) error {
return err
}

if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
fmt.Fprintf(w, "\n")

res := &plainType{
Expand Down
8 changes: 4 additions & 4 deletions pkg/codegen/go/gen.go
Expand Up @@ -2110,7 +2110,7 @@ func (pkg *pkgContext) genFunction(w io.Writer, f *schema.Function) error {
argsig = fmt.Sprintf("%s, args *%sArgs", argsig, name)
}
var retty string
if f.Outputs == nil {
if f.Outputs == nil || len(f.Outputs.Properties) == 0 {
retty = "error"
} else {
retty = fmt.Sprintf("(*%sResult, error)", name)
Expand All @@ -2129,7 +2129,7 @@ func (pkg *pkgContext) genFunction(w io.Writer, f *schema.Function) error {

// Now simply invoke the runtime function with the arguments.
var outputsType string
if f.Outputs == nil {
if f.Outputs == nil || len(f.Outputs.Properties) == 0 {
outputsType = "struct{}"
} else {
outputsType = name + "Result"
Expand All @@ -2143,7 +2143,7 @@ func (pkg *pkgContext) genFunction(w io.Writer, f *schema.Function) error {
fmt.Fprintf(w, "\tvar rv %s\n", outputsType)
fmt.Fprintf(w, "\terr := ctx.Invoke(\"%s\", %s, &rv, opts...)\n", f.Token, inputsVar)

if f.Outputs == nil {
if f.Outputs == nil || len(f.Outputs.Properties) == 0 {
fmt.Fprintf(w, "\treturn err\n")
} else {
// Check the error before proceeding.
Expand Down Expand Up @@ -2173,7 +2173,7 @@ func (pkg *pkgContext) genFunction(w io.Writer, f *schema.Function) error {
}
}
}
if f.Outputs != nil {
if f.Outputs != nil && len(f.Outputs.Properties) > 0 {
fmt.Fprintf(w, "\n")
fnOutputsName := pkg.functionResultTypeName(f)
pkg.genPlainType(w, fnOutputsName, f.Outputs.Comment, "", f.Outputs.Properties)
Expand Down
4 changes: 2 additions & 2 deletions pkg/codegen/nodejs/gen.go
Expand Up @@ -1146,7 +1146,7 @@ func (mod *modContext) genFunction(w io.Writer, fun *schema.Function) (functionF
}
info.functionArgsInterfaceName = argsInterfaceName
}
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
fmt.Fprintf(w, "\n")
resultInterfaceName := title(name) + "Result"
if err := mod.genPlainType(w, resultInterfaceName, fun.Outputs.Comment, fun.Outputs.Properties, false, true, 0); err != nil {
Expand All @@ -1170,7 +1170,7 @@ func functionArgsOptional(fun *schema.Function) bool {
}

func functionReturnType(fun *schema.Function) string {
if fun.Outputs == nil {
if fun.Outputs == nil || len(fun.Outputs.Properties) == 0 {
return "void"
}
return title(tokenToFunctionName(fun.Token)) + "Result"
Expand Down
69 changes: 48 additions & 21 deletions pkg/codegen/python/gen.go
Expand Up @@ -365,29 +365,54 @@ func genStandardHeader(w io.Writer, tool string) {
fmt.Fprintf(w, "# *** Do not edit by hand unless you're certain you know what you are doing! ***\n\n")
}

func typingImports() []string {
return []string{
"Any",
"Mapping",
"Optional",
"Sequence",
"Union",
"overload",
}
}

func (mod *modContext) generateCommonImports(w io.Writer, imports imports, typingImports []string) {
rel, err := filepath.Rel(mod.mod, "")
contract.Assert(err == nil)
relRoot := path.Dir(rel)
relImport := relPathToRelImport(relRoot)

fmt.Fprintf(w, "import copy\n")
fmt.Fprintf(w, "import warnings\n")
fmt.Fprintf(w, "import pulumi\n")
fmt.Fprintf(w, "import pulumi.runtime\n")
fmt.Fprintf(w, "from typing import %s\n", strings.Join(typingImports, ", "))
fmt.Fprintf(w, "from %s import _utilities\n", relImport)
for _, imp := range imports.strings() {
fmt.Fprintf(w, "%s\n", imp)
}
fmt.Fprintf(w, "\n")
}

func (mod *modContext) genHeader(w io.Writer, needsSDK bool, imports imports) {
genStandardHeader(w, mod.tool)

// If needed, emit the standard Pulumi SDK import statement.
if needsSDK {
rel, err := filepath.Rel(mod.mod, "")
contract.Assert(err == nil)
relRoot := path.Dir(rel)
relImport := relPathToRelImport(relRoot)

fmt.Fprintf(w, "import copy\n")
fmt.Fprintf(w, "import warnings\n")
fmt.Fprintf(w, "import pulumi\n")
fmt.Fprintf(w, "import pulumi.runtime\n")
fmt.Fprintf(w, "from typing import Any, Mapping, Optional, Sequence, Union, overload\n")
fmt.Fprintf(w, "from %s import _utilities\n", relImport)
for _, imp := range imports.strings() {
fmt.Fprintf(w, "%s\n", imp)
}
fmt.Fprintf(w, "\n")
typings := typingImports()
mod.generateCommonImports(w, imports, typings)
}
}

func (mod *modContext) genFunctionHeader(w io.Writer, function *schema.Function, imports imports) {
genStandardHeader(w, mod.tool)
typings := typingImports()
if function.Outputs == nil || len(function.Outputs.Properties) == 0 {
typings = append(typings, "Awaitable")
}
mod.generateCommonImports(w, imports, typings)
}

func relPathToRelImport(relPath string) string {
// Convert relative path to relative import e.g. "../.." -> "..."
// https://realpython.com/absolute-vs-relative-python-imports/#relative-imports
Expand Down Expand Up @@ -1696,17 +1721,17 @@ func (mod *modContext) genFunction(fun *schema.Function) (string, error) {
mod.collectImports(fun.Outputs.Properties, imports, false)
}

mod.genHeader(w, true /*needsSDK*/, imports)
mod.genFunctionHeader(w, fun, imports)

var baseName, awaitableName string
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
baseName, awaitableName = awaitableTypeNames(fun.Outputs.Token)
}
name := PyName(tokenToName(fun.Token))

// Export only the symbols we want exported.
fmt.Fprintf(w, "__all__ = [\n")
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
fmt.Fprintf(w, " '%s',\n", baseName)
fmt.Fprintf(w, " '%s',\n", awaitableName)
}
Expand All @@ -1724,9 +1749,11 @@ func (mod *modContext) genFunction(fun *schema.Function) (string, error) {
// If there is a return type, emit it.
retTypeName := ""
var rets []*schema.Property
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
retTypeName, rets = mod.genAwaitableType(w, fun.Outputs), fun.Outputs.Properties
fmt.Fprintf(w, "\n\n")
} else {
retTypeName = "Awaitable[None]"
}

var args []*schema.Property
Expand All @@ -1750,7 +1777,7 @@ func (mod *modContext) genFunction(fun *schema.Function) (string, error) {

// Now simply invoke the runtime function with the arguments.
var typ string
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
// Pass along the private output_type we generated, so any nested outputs classes are instantiated by
// the call to invoke.
typ = fmt.Sprintf(", typ=%s", baseName)
Expand All @@ -1759,7 +1786,7 @@ func (mod *modContext) genFunction(fun *schema.Function) (string, error) {
fmt.Fprintf(w, "\n")

// And copy the results to an object, if there are indeed any expected returns.
if fun.Outputs != nil {
if fun.Outputs != nil && len(fun.Outputs.Properties) > 0 {
fmt.Fprintf(w, " return %s(", retTypeName)
for i, ret := range rets {
if i > 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/codegen/schema/schema.go
Expand Up @@ -561,7 +561,7 @@ func (fun *Function) NeedsOutputVersion() bool {
// support them and return `Task`, but there are no such
// functions in `pulumi-azure-native` or `pulumi-aws` so we
// omit to simplify.
if fun.Outputs == nil {
if fun.Outputs == nil || len(fun.Outputs.Properties) == 0 {
return false
}

Expand Down
Expand Up @@ -19,11 +19,6 @@ n/a

## Using funcWithEmptyOutputs {#using}

Two invocation forms are available. The direct form accepts plain
arguments and either blocks until the result value is available, or
returns a Promise-wrapped result. The output form accepts
Input-wrapped arguments and returns an Output-wrapped result.

<div>
<pulumi-chooser type="language" options="typescript,python,go,csharp,java,yaml"></pulumi-chooser>
</div>
Expand All @@ -34,8 +29,6 @@ Input-wrapped arguments and returns an Output-wrapped result.
<div class="highlight"
><pre class="chroma"><code class="language-typescript" data-lang="typescript"
><span class="k">function </span>funcWithEmptyOutputs<span class="p">(</span><span class="nx">args</span><span class="p">:</span> <span class="nx">FuncWithEmptyOutputsArgs</span><span class="p">,</span> <span class="nx">opts</span><span class="p">?:</span> <span class="nx"><a href="/docs/reference/pkg/nodejs/pulumi/pulumi/#InvokeOptions">InvokeOptions</a></span><span class="p">): Promise&lt;<span class="nx"><a href="#result">FuncWithEmptyOutputsResult</a></span>></span
><span class="k">
function </span>funcWithEmptyOutputsOutput<span class="p">(</span><span class="nx">args</span><span class="p">:</span> <span class="nx">FuncWithEmptyOutputsOutputArgs</span><span class="p">,</span> <span class="nx">opts</span><span class="p">?:</span> <span class="nx"><a href="/docs/reference/pkg/nodejs/pulumi/pulumi/#InvokeOptions">InvokeOptions</a></span><span class="p">): Output&lt;<span class="nx"><a href="#result">FuncWithEmptyOutputsResult</a></span>></span
></code></pre></div>
</pulumi-choosable>
</div>
Expand All @@ -46,9 +39,6 @@ function </span>funcWithEmptyOutputsOutput<span class="p">(</span><span class="n
<div class="highlight"><pre class="chroma"><code class="language-python" data-lang="python"
><span class="k">def </span>func_with_empty_outputs<span class="p">(</span><span class="nx">name</span><span class="p">:</span> <span class="nx">Optional[str]</span> = None<span class="p">,</span>
<span class="nx">opts</span><span class="p">:</span> <span class="nx"><a href="/docs/reference/pkg/python/pulumi/#pulumi.InvokeOptions">Optional[InvokeOptions]</a></span> = None<span class="p">) -&gt;</span> <span>FuncWithEmptyOutputsResult</span
><span class="k">
def </span>func_with_empty_outputs_output<span class="p">(</span><span class="nx">name</span><span class="p">:</span> <span class="nx">Optional[pulumi.Input[str]]</span> = None<span class="p">,</span>
<span class="nx">opts</span><span class="p">:</span> <span class="nx"><a href="/docs/reference/pkg/python/pulumi/#pulumi.InvokeOptions">Optional[InvokeOptions]</a></span> = None<span class="p">) -&gt;</span> <span>Output[FuncWithEmptyOutputsResult]</span
></code></pre></div>
</pulumi-choosable>
</div>
Expand All @@ -58,8 +48,6 @@ def </span>func_with_empty_outputs_output<span class="p">(</span><span class="nx
<pulumi-choosable type="language" values="go">
<div class="highlight"><pre class="chroma"><code class="language-go" data-lang="go"
><span class="k">func </span>FuncWithEmptyOutputs<span class="p">(</span><span class="nx">ctx</span><span class="p"> *</span><span class="nx"><a href="https://pkg.go.dev/github.com/pulumi/pulumi/sdk/v3/go/pulumi?tab=doc#Context">Context</a></span><span class="p">,</span> <span class="nx">args</span><span class="p"> *</span><span class="nx">FuncWithEmptyOutputsArgs</span><span class="p">,</span> <span class="nx">opts</span><span class="p"> ...</span><span class="nx"><a href="https://pkg.go.dev/github.com/pulumi/pulumi/sdk/v3/go/pulumi?tab=doc#InvokeOption">InvokeOption</a></span><span class="p">) (*<span class="nx"><a href="#result">FuncWithEmptyOutputsResult</a></span>, error)</span
><span class="k">
func </span>FuncWithEmptyOutputsOutput<span class="p">(</span><span class="nx">ctx</span><span class="p"> *</span><span class="nx"><a href="https://pkg.go.dev/github.com/pulumi/pulumi/sdk/v3/go/pulumi?tab=doc#Context">Context</a></span><span class="p">,</span> <span class="nx">args</span><span class="p"> *</span><span class="nx">FuncWithEmptyOutputsOutputArgs</span><span class="p">,</span> <span class="nx">opts</span><span class="p"> ...</span><span class="nx"><a href="https://pkg.go.dev/github.com/pulumi/pulumi/sdk/v3/go/pulumi?tab=doc#InvokeOption">InvokeOption</a></span><span class="p">) FuncWithEmptyOutputsResultOutput</span
></code></pre></div>
&gt; Note: This function is named `FuncWithEmptyOutputs` in the Go SDK.
Expand All @@ -72,8 +60,7 @@ func </span>FuncWithEmptyOutputsOutput<span class="p">(</span><span class="nx">c
<pulumi-choosable type="language" values="csharp">
<div class="highlight"><pre class="chroma"><code class="language-csharp" data-lang="csharp"><span class="k">public static class </span><span class="nx">FuncWithEmptyOutputs </span><span class="p">
{</span><span class="k">
public static </span>Task&lt;<span class="nx"><a href="#result">FuncWithEmptyOutputsResult</a></span>> <span class="p">InvokeAsync(</span><span class="nx">FuncWithEmptyOutputsArgs</span><span class="p"> </span><span class="nx">args<span class="p">,</span> <span class="nx"><a href="/docs/reference/pkg/dotnet/Pulumi/Pulumi.InvokeOptions.html">InvokeOptions</a></span><span class="p">? </span><span class="nx">opts = null<span class="p">)</span><span class="k">
public static </span>Output&lt;<span class="nx"><a href="#result">FuncWithEmptyOutputsResult</a></span>> <span class="p">Invoke(</span><span class="nx">FuncWithEmptyOutputsInvokeArgs</span><span class="p"> </span><span class="nx">args<span class="p">,</span> <span class="nx"><a href="/docs/reference/pkg/dotnet/Pulumi/Pulumi.InvokeOptions.html">InvokeOptions</a></span><span class="p">? </span><span class="nx">opts = null<span class="p">)</span><span class="p">
public static </span>Task&lt;<span class="nx"><a href="#result">FuncWithEmptyOutputsResult</a></span>> <span class="p">InvokeAsync(</span><span class="nx">FuncWithEmptyOutputsArgs</span><span class="p"> </span><span class="nx">args<span class="p">,</span> <span class="nx"><a href="/docs/reference/pkg/dotnet/Pulumi/Pulumi.InvokeOptions.html">InvokeOptions</a></span><span class="p">? </span><span class="nx">opts = null<span class="p">)</span><span class="p">
}</span></code></pre></div>
</pulumi-choosable>
</div>
Expand Down
Expand Up @@ -14,14 +14,8 @@ public static class FuncWithEmptyOutputs
/// <summary>
/// n/a
/// </summary>
public static Task<FuncWithEmptyOutputsResult> InvokeAsync(FuncWithEmptyOutputsArgs args, InvokeOptions? options = null)
=> global::Pulumi.Deployment.Instance.InvokeAsync<FuncWithEmptyOutputsResult>("mypkg::funcWithEmptyOutputs", args ?? new FuncWithEmptyOutputsArgs(), options.WithDefaults());

/// <summary>
/// n/a
/// </summary>
public static Output<FuncWithEmptyOutputsResult> Invoke(FuncWithEmptyOutputsInvokeArgs args, InvokeOptions? options = null)
=> global::Pulumi.Deployment.Instance.Invoke<FuncWithEmptyOutputsResult>("mypkg::funcWithEmptyOutputs", args ?? new FuncWithEmptyOutputsInvokeArgs(), options.WithDefaults());
public static Task InvokeAsync(FuncWithEmptyOutputsArgs args, InvokeOptions? options = null)
=> global::Pulumi.Deployment.Instance.InvokeAsync("mypkg::funcWithEmptyOutputs", args ?? new FuncWithEmptyOutputsArgs(), options.WithDefaults());
}


Expand All @@ -38,28 +32,4 @@ public FuncWithEmptyOutputsArgs()
}
public static new FuncWithEmptyOutputsArgs Empty => new FuncWithEmptyOutputsArgs();
}

public sealed class FuncWithEmptyOutputsInvokeArgs : global::Pulumi.InvokeArgs
{
/// <summary>
/// The Name of the FeatureGroup.
/// </summary>
[Input("name", required: true)]
public Input<string> Name { get; set; } = null!;

public FuncWithEmptyOutputsInvokeArgs()
{
}
public static new FuncWithEmptyOutputsInvokeArgs Empty => new FuncWithEmptyOutputsInvokeArgs();
}


[OutputType]
public sealed class FuncWithEmptyOutputsResult
{
[OutputConstructor]
private FuncWithEmptyOutputsResult()
{
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c11a8eb

Please sign in to comment.