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

[codegen/python] Support <Resource>Args classes #6525

Merged
merged 4 commits into from
Apr 2, 2021
Merged

Conversation

justinvp
Copy link
Member

@justinvp justinvp commented Mar 13, 2021

Add support for creating instances of resources in Python using a <Resource>Args class. This capability aligns with how args are passed to resources in all the other language SDKs and the separate object bag allows the properties to be manipulated/validated/passed-around before creating the resource.

Part of #5146

@github-actions
Copy link

Diff for pulumi-random with merge commit 1c9cc8d

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 1c9cc8d

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 1c9cc8d

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 1c9cc8d

@github-actions
Copy link

Diff for pulumi-azure with merge commit 1c9cc8d

@github-actions
Copy link

Diff for pulumi-aws with merge commit 1c9cc8d

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 1c9cc8d

@justinvp justinvp added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Mar 15, 2021
@justinvp justinvp marked this pull request as ready for review March 15, 2021 18:03
@github-actions
Copy link

Diff for pulumi-random with merge commit 6896908

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 6896908

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 6896908

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 6896908

@github-actions
Copy link

Diff for pulumi-azure with merge commit 6896908

@github-actions
Copy link

Diff for pulumi-aws with merge commit 6896908

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 6896908

mod.genInitDocstring(w, res)
// Now generate __init__ overloads along with an implementation...

// First, generate an __init__ overload that accepts the resource's inputs from the args class.
Copy link
Member Author

@justinvp justinvp Mar 15, 2021

Choose a reason for hiding this comment

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

Do we have a preference regarding which overload is shown first in code completion lists? VS Code shows the overloads in the order they're declared (at least in this case), which is currently the new overload.

Screen Shot 2021-03-15 at 12 59 03 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd propose having the keyword-based style first ahead of the args-based given that's what people are currently using. We could choose to flip this order for 3.0 to encourage the args-based as the 'default'

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM. I've pushed a commit that does that.

Screen Shot 2021-03-16 at 6 22 18 AM

Screen Shot 2021-03-16 at 6 22 41 AM

@justinvp justinvp requested a review from komalali March 15, 2021 20:05
@github-actions
Copy link

Diff for pulumi-random with merge commit c764181

@github-actions
Copy link

Diff for pulumi-azuread with merge commit c764181

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit c764181

@github-actions
Copy link

Diff for pulumi-gcp with merge commit c764181

@github-actions
Copy link

Diff for pulumi-azure with merge commit c764181

@github-actions
Copy link

Diff for pulumi-aws with merge commit c764181

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit c764181

Copy link
Member

@komalali komalali left a comment

Choose a reason for hiding this comment

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

LGTM. Do we know what impact this will have on docs and how we will handle presenting the overloads there?

@github-actions
Copy link

Diff for pulumi-azuread with merge commit c7fb9da

@github-actions
Copy link

Diff for pulumi-random with merge commit c7fb9da

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit c7fb9da

@github-actions
Copy link

Diff for pulumi-gcp with merge commit c7fb9da

@github-actions
Copy link

Diff for pulumi-azure with merge commit c7fb9da

@github-actions
Copy link

Diff for pulumi-aws with merge commit c7fb9da

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit c7fb9da

@komalali
Copy link
Member

Oh, I realized that all the tests here are of the codegen itself. Could we add some tests of this in use?

Add support for creating instances of resources in Python using a
`<Resource>Args` class. This capability aligns with how args are passed
to resources in all the other language SDKs and the separate object bag
allows the properties to be manipulated/validated/passed-around before
creating the resource.
Emit the overload that accepts the new args class last, so code completion shows the existing function-arg-based overload first.
Fix a regression that was causing function inputs to be incorrectly wrapped as `Input[T]`.
@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-random with merge commit bc61f9e

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-kubernetes with merge commit bc61f9e

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-azuread with merge commit bc61f9e

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-gcp with merge commit bc61f9e

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-azure with merge commit bc61f9e

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-aws with merge commit bc61f9e

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

Diff for pulumi-azure-native with merge commit bc61f9e

@justinvp justinvp merged commit dcf4359 into master Apr 2, 2021
@pulumi-bot pulumi-bot deleted the justin/pyargs branch April 2, 2021 17:09
lblackstone added a commit to pulumi/pulumi-kubernetes that referenced this pull request Apr 8, 2021
 - [sdk/go] Fix Go resource registrations (pulumi/pulumi#6641)
 - [sdk/python] Support `<Resource>Args` classes (pulumi/pulumi#6525)
lblackstone added a commit to pulumi/pulumi-kubernetes that referenced this pull request Apr 14, 2021
 - [sdk/go] Fix Go resource registrations (pulumi/pulumi#6641)
 - [sdk/python] Support `<Resource>Args` classes (pulumi/pulumi#6525)
lblackstone added a commit to pulumi/pulumi-kubernetes that referenced this pull request Apr 14, 2021
 - [sdk/go] Fix Go resource registrations (pulumi/pulumi#6641)
 - [sdk/python] Support `<Resource>Args` classes (pulumi/pulumi#6525)
lblackstone added a commit to pulumi/pulumi-kubernetes that referenced this pull request Apr 15, 2021
- [sdk/go] Fix Go resource registrations (pulumi/pulumi#6641)
 - [sdk/python] Support `<Resource>Args` classes (pulumi/pulumi#6525)
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.

None yet

3 participants