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
[program-gen] Fix panic when generating programs for MLC packages using external types #15605
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Zaid-Ajaj
added
language/python
language/go
language/dotnet
area/codegen
SDK-gen, program-gen, convert
labels
Mar 6, 2024
Changelog[uncommitted] (2024-03-10)Bug Fixes
|
This was referenced Mar 6, 2024
Zaid-Ajaj
force-pushed
the
zaid/aws-static-website-panic
branch
from
March 6, 2024 16:42
28f953e
to
2431725
Compare
This was referenced Mar 6, 2024
Zaid-Ajaj
force-pushed
the
zaid/aws-static-website-panic
branch
from
March 6, 2024 17:12
050de6b
to
dfa130e
Compare
This was referenced Mar 6, 2024
justinvp
approved these changes
Mar 9, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Just need to update the test data to react to #15549
Zaid-Ajaj
force-pushed
the
zaid/aws-static-website-panic
branch
from
March 10, 2024 04:24
2af8f15
to
8b79d09
Compare
This was referenced Mar 10, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Mar 10, 2024
…e type references to external packages
Zaid-Ajaj
force-pushed
the
zaid/aws-static-website-panic
branch
from
March 10, 2024 16:57
7cb775f
to
94f6d02
Compare
This was referenced Mar 10, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 14, 2024
Tentative changelog: ### Features - [sdk/python] Add experimental support to the Python SDK for the new transforms system [#15376](#15376) ### Bug Fixes - [cli/state] Add `--yes` to `state upgrade` [#15648](#15648) - [programgen/{dotnet,go,python}] Fix panic when generating programs for MLC packages where they include type references to external packages [#15605](#15605) - [programgen/go] Fix optional primitive values being derefrenced [#15592](#15592) - [sdk/go] Await output properties from Construct/Call before closing the Context [#15611](#15611) - [sdk/nodejs] Fix codepaths computation when working dir is nested relative to package.json [#15601](#15601) - [sdk/nodejs] Replace glob with fdir to avoid an indirect dependency on inflight [#15617](#15617)
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 14, 2024
### Features - [cli] Make "pulumi dn" an alias for "pulumi destroy" [#15650](#15650) - [yaml] Update yaml to v1.6.0 [#15661](#15661) - [sdk/python] Add experimental support to the Python SDK for the new transforms system [#15376](#15376) ### Bug Fixes - [cli/state] Add `--yes` to `state upgrade` [#15648](#15648) - [programgen/{dotnet,go,python}] Fix panic when generating programs for MLC packages where they include type references to external packages [#15605](#15605) - [programgen/go] Fix optional primitive values being derefrenced [#15592](#15592) - [sdk/go] Await output properties from Construct/Call before closing the Context [#15611](#15611) - [sdk/nodejs] Fix codepaths computation when working dir is nested relative to package.json [#15601](#15601) - [sdk/nodejs] Replace glob with fdir to avoid an indirect dependency on inflight [#15617](#15617) - [sdkgen/python] Make replace-on-changes values camelCased not kebab_cased [#15666](#15666)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
For an MLC package such as
aws-static-website
, it has some types which are referenced from theaws
package. Program-gen assumes packages are inferred from resources and invokes, not types which caused panics in Go (#15597), dotnet and python (pulumi/pulumi-converter-constructor-syntax#2)This PR fixes those panics. In Go the panic was due to using package name instead of the package reference from the imported type. In dotnet and python was due to assuming no external type references. Now we generate nice code for all these languages.
That said, there is still an issue of resolving imports for the packages of these external types. It works in Go, TypeScript doesn't need it but dotnet and python do. That is why the latter are added in
SkipCompile
in the test program.Fixes #15597
Fixes pulumi/pulumi-converter-constructor-syntax#3
Fixes pulumi/pulumi-converter-constructor-syntax#2
Checklist
make tidy
to update any new dependenciesmake lint
to verify my code passes the lint checkgofumpt
make changelog
and committed thechangelog/pending/<file>
documenting my change