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

Reify Input and Optional types in the schema type system. #7059

Merged
merged 11 commits into from
Jun 24, 2021

Conversation

pgavlin
Copy link
Member

@pgavlin pgavlin commented May 14, 2021

These changes support arbitrary combinations of input + plain types
within a schema. Handling plain types at the property level was not
sufficient to support such combinations. Reifying these types
required updating quite a bit of code. This is likely to have caused
some temporary complications, but should eventually lead to
substantial simplification in the SDK and program code generators.

With the new design, input and optional types are explicit in the schema
type system. Optionals will only appear at the outermost level of a type
(i.e. Input<Optional<>>, Array<Optional<>>, etc. will not occur). In
addition to explicit input types, each object type now has a "plain"
shape and an "input" shape. The former uses only plain types; the latter
uses input shapes wherever a plain type is not specified. Plain types
are indicated in the schema by setting the "plain" property of a type spec
to true.

@github-actions
Copy link

Diff for pulumi-random with merge commit 3230abb

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 3230abb

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 3230abb

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 3230abb

@github-actions
Copy link

Diff for pulumi-azure with merge commit 3230abb

@github-actions
Copy link

Diff for pulumi-aws with merge commit 3230abb

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 3230abb

@pgavlin
Copy link
Member Author

pgavlin commented May 14, 2021

@cnunciato it looks like a bunch of docs tests changed here--any idea why? I didn't touch anything in the docs generator directly...

@github-actions
Copy link

Diff for pulumi-random with merge commit 7a8e5a2

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 7a8e5a2

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 7a8e5a2

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 7a8e5a2

@github-actions
Copy link

Diff for pulumi-azure with merge commit 7a8e5a2

@cnunciato
Copy link
Member

@pgavlin Not immediately sure either. I don't see this on master, but I do see it when I check out your branch and run the tests with PULUMI_ASSERT. Anything in your branch that would affect template rendering?

@github-actions
Copy link

Diff for pulumi-aws with merge commit 7a8e5a2

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 7a8e5a2

@pgavlin
Copy link
Member Author

pgavlin commented May 14, 2021

Anything in your branch that would affect template rendering?

There shouldn't be, no. The changes I made should be localized to SDK generation.

@t0yv0
Copy link
Member

t0yv0 commented May 14, 2021

Where do we define shallow/deep unwrapping? Looking at this PR, Foo and FooArgs in the example seem structurally identical. Would a difference appear if they referenced nested Output/Input props?

@jen20
Copy link
Contributor

jen20 commented May 18, 2021

This does not address the issue for Go, where plain container types still generate plain args classes.

Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

Something isn't right with these changes... I tried generating SDKs with these changes with the following schema:

{
    "name": "xyz",
    "types": {
        "example:index:Foo": {
          "properties": {
            "a": {
              "type": "boolean"
            }
          },
          "type": "object"
        }
      },
    "resources": {
        "xyz:index:StaticPage": {
            "isComponent": true,
            "inputProperties": {
                "indexContent": {
                    "type": "string",
                    "description": "The HTML content for index.html."
                },
                "foo": {
                    "$ref": "#/types/example:index:Foo"
                }
            },
            "requiredInputs": [
                "indexContent"
            ],
            "properties": {
                "bucket": {
                    "$ref": "/aws/v4.0.0/schema.json#/resources/aws:s3%2Fbucket:Bucket",
                    "description": "The bucket resource."
                },
                "websiteUrl": {
                    "type": "string",
                    "description": "The website URL."
                }
            },
            "required": [
                "bucket",
                "websiteUrl"
            ],
            "plainInputs": ["foo"]
        }
    },
    "language": {
        "csharp": {
            "packageReferences": {
                "Pulumi": "3.*",
                "Pulumi.Aws": "4.*"
            }
        },
        "go": {
            "generateResourceContainerTypes": true,
            "importBasePath": "github.com/pulumi/pulumi-xyz/sdk/go/xyz"
        },
        "nodejs": {
            "dependencies": {
                "@pulumi/aws": "^4.0.0"
            },
            "devDependencies": {
                "typescript": "^3.7.0"
            }
        },
        "python": {
            "requires": {
                "pulumi": ">=3.0.0,<4.0.0",
                "pulumi-aws": ">=4.0.0,<5.0.0"
            }
        }
    }
}

.NET

The resource args refers to FooArgs:

public sealed class StaticPageArgs : Pulumi.ResourceArgs
{
    [Input("foo")]
    public Pulumi.Example.Inputs.FooArgs? Foo { get; set; }

    /// <summary>
    /// The HTML content for index.html.
    /// </summary>
    [Input("indexContent", required: true)]
    public Input<string> IndexContent { get; set; } = null!;

    public StaticPageArgs()
    {
    }
}

And sdk/dotnet/Inputs/FooArgs.cs is emitted, but it's empty (missing definition for FooArgs):

// *** WARNING: this file was generated by Pulumi SDK Generator. ***
// *** Do not edit by hand unless you're certain you know what you are doing! ***

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading.Tasks;
using Pulumi.Serialization;

namespace Pulumi.Example.Inputs
{
}

Node.js

Similarly, the resource's args refers to the FooArgs correctly:

export interface StaticPageArgs {
    foo?: inputs.FooArgs;
    /**
     * The HTML content for index.html.
     */
    indexContent: pulumi.Input<string>;
}

And sdk/nodejs/types/input.ts is emitted, but it's empty (missing definition for FooArgs):

// *** WARNING: this file was generated by Pulumi SDK Generator. ***
// *** Do not edit by hand unless you're certain you know what you are doing! ***

import * as pulumi from "@pulumi/pulumi";
import { input as inputs, output as outputs } from "../types";

Python

Similarly, the resource args refers to FooArgs:

class StaticPage(pulumi.ComponentResource):
    @overload
    def __init__(__self__,
                 resource_name: str,
                 opts: Optional[pulumi.ResourceOptions] = None,
                 foo: Optional[pulumi.InputType['FooArgs']] = None,
                 index_content: Optional[pulumi.Input[str]] = None,
                 __props__=None):

And sdk/python/pulumi_xyz/_inputs.py is emitted, but it's empty (missing definition of FooArgs):

# coding=utf-8
# *** WARNING: this file was generated by Pulumi SDK Generator. ***
# *** Do not edit by hand unless you're certain you know what you are doing! ***

import warnings
import pulumi
import pulumi.runtime
from typing import Any, Mapping, Optional, Sequence, Union, overload
from . import _utilities

__all__ = [
]

@github-actions
Copy link

Diff for pulumi-random with merge commit c64334f

@github-actions
Copy link

Diff for pulumi-azuread with merge commit c64334f

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit c64334f

@github-actions
Copy link

Diff for pulumi-gcp with merge commit c64334f

@pgavlin
Copy link
Member Author

pgavlin commented May 25, 2021

This does not address the issue for Go, where plain container types still generate plain args classes.

This is now done.

Something isn't right with these changes... I tried generating SDKs with these changes with the following schema:

Excellent catch, @justinvp. Fixed.

@github-actions
Copy link

Diff for pulumi-azure with merge commit c64334f

@github-actions
Copy link

Diff for pulumi-aws with merge commit c64334f

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 80802c8

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 80802c8

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 80802c8

@github-actions
Copy link

Diff for pulumi-azure with merge commit 80802c8

@github-actions
Copy link

Diff for pulumi-aws with merge commit 80802c8

@pgavlin pgavlin added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Jun 24, 2021
@pgavlin pgavlin merged commit 7b1d6ec into master Jun 24, 2021
@pulumi-bot pulumi-bot deleted the pgavlin/gh6957 branch June 24, 2021 16:17
justinvp added a commit to pulumi/pulumi-hugo that referenced this pull request Aug 18, 2021
React to changes from pulumi/pulumi#7059

 - Remove `plain` and `plainInputs` array properties
 - Add a bool `plain` property to `Type`.
justinvp added a commit to pulumi/pulumi-hugo that referenced this pull request Aug 19, 2021
React to changes from pulumi/pulumi#7059

 - Remove `plain` and `plainInputs` array properties
 - Add a bool `plain` property to `Type`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants