-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[engine/import] Guess ID references of dependant resources when generating code for import operations #16208
Conversation
Changelog[uncommitted] (2024-05-20)Features
|
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.
What if multiple resources have the same ID? Do we just pick one randomly, or fall back to the literal string?
I didn't think about that 🤔 right now this will pick the first ID that was detected. I think if we want to be deterministic, we would have to remove ambiguous references to the same ID and the program would fall back to the literal value. However, if this is a highly unlikely event, we can maybe keep it as is. |
d351184
to
9d873a6
Compare
bdda341
to
7db586a
Compare
@Frassle I've updated the PR such that now when multiple resources have the same ID, we don't make a reference to any of them and instead use the literal value instead. More generally, pathed values with different paths but equal values are removed. This will work nicely without any ambiguity when we extend the pathed values not just for |
pkg/importer/hcl2.go
Outdated
@@ -307,6 +353,8 @@ func simplerType(t, u schema.Type) bool { | |||
|
|||
// zeroValue constructs a zero value of the given type. | |||
func zeroValue(t schema.Type) model.Expression { | |||
emptyImportState := ImportState{} | |||
onRefereceAdded := func(string) {} |
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.
onRefereceAdded := func(string) {} | |
onReferenceAdded := func(string) {} |
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.
Done ✅
9d7b6a5
to
4c4d51c
Compare
Tentative changelog: ### Features - [engine] Guess ID references of dependant resources when generating code for import operations [#16208](#16208) ### Bug Fixes - [engine] Check property dependencies and deleted-with relationships for target dependents [#16220](#16220) - [engine] Propagate dependencies of untargeted resources correctly during targeted updates [#16247](#16247) - [backend/diy] Rewrite DeletedWith references when renaming stacks [#16216](#16216) - [sdk/python] Use a separate type variable for static methods on Output [#16172](#16172) - [sdk/python] Relax Output.all types to better match the implementation [#16172](#16172) - [sdkgen/python] Generate __init__.py files for modules that only contain enumerations [#16229](#16229)
To be merged after: - #16261 - pulumi/pulumi-docker-containers#195 Tentative changelog... ### Features - [engine] Guess ID references of dependant resources when generating code for import operations [#16208](#16208) ### Bug Fixes - [engine] Check property dependencies and deleted-with relationships for target dependents [#16220](#16220) - [engine] Propagate dependencies of untargeted resources correctly during targeted updates [#16247](#16247) - [backend/diy] Rewrite DeletedWith references when renaming stacks [#16216](#16216) - [cli/state] Fix state renames involving DeletedWith - [sdk/python] Use a separate type variable for static methods on Output [#16172](#16172) - [sdk/python] Relax Output.all types to better match the implementation [#16172](#16172) - [sdkgen/python] Generate __init__.py files for modules that only contain enumerations [#16229](#16229)
Description
Taking an initial attempt at #10025 where we now try to guess ID references of dependant resources instead of writing out the IDs as literal values.
Instead of:
We generate:
This implies a dependency between
exampleBucketObject
andexampleBucket
without explicitly being added into thedependsOn
array (it would be redundant)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