-
Notifications
You must be signed in to change notification settings - Fork 50
Fix nested computed read #2181
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
Merged
Merged
Fix nested computed read #2181
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
0c07814
fix nested compute read
VenelinMartinov 841998f
fix tests = ()
VenelinMartinov a2b4bf5
remove debug logging
VenelinMartinov baa894f
integraration tests
VenelinMartinov efe595d
fix object check in input extraction
VenelinMartinov 96a2f0b
add further todos related to wrong object check
VenelinMartinov 48fd41b
Revert "add further todos related to wrong object check"
VenelinMartinov 41499a0
revert logging change
VenelinMartinov 637504a
lint
VenelinMartinov 7e5b053
update test
VenelinMartinov ff29def
upgrade providertest package
VenelinMartinov bf92dfd
go mod tidy
VenelinMartinov ec38a62
Merge branch 'master' into vvm/fix_nested_computed_read1
VenelinMartinov 4399511
remove note about CastToTypeObject
VenelinMartinov f0a277b
add note to CastToTypeObject
VenelinMartinov 0260a54
remove unused pulumi schema override
VenelinMartinov 886d4f2
Merge branch 'master' into vvm/fix_nested_computed_read1
VenelinMartinov 4d31021
make tidy
VenelinMartinov f9ce200
Merge branch 'master' into vvm/fix_nested_computed_read1
VenelinMartinov 76191b9
remove unused schema options
VenelinMartinov 32f9e37
Merge branch 'vvm/remove_unused_schema_options' into vvm/fix_nested_c…
VenelinMartinov 39251cf
pf extract inputs from outputs tests
VenelinMartinov b5ce289
map tests
VenelinMartinov e40f6a0
more tests
VenelinMartinov 4a59ef9
rename tests, add block tests
VenelinMartinov ec3ae31
Merge branch 'vvm/pf_extract_inputs_from_outputs' into vvm/fix_nested…
VenelinMartinov 1b672b5
fix pf tests
VenelinMartinov 7fac4a1
add more todos
VenelinMartinov f07cf0c
fix todos
VenelinMartinov 8f9d753
Merge branch 'vvm/pf_extract_inputs_from_outputs' into vvm/fix_nested…
VenelinMartinov 4427b72
Merge branch 'master' into vvm/pf_extract_inputs_from_outputs
VenelinMartinov 849961e
Merge branch 'vvm/pf_extract_inputs_from_outputs' into vvm/fix_nested…
VenelinMartinov 08fc395
fix sdkv2 tests
VenelinMartinov 89906ef
remove note
VenelinMartinov 13521d8
Merge branch 'master' into vvm/fix_nested_computed_read1
VenelinMartinov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
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.
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.
This is exactly the root cause, I find it unfortunate to remove this comment.
elemSchemasproducesThis is not a valid schema as it does not have
Type()defined. This is an artificial schema that you will not find coming from the actual provider, it's a recursion artifact.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.
But CastToTypeObject is wrong in the first place for SDKv2 - it checks for .Resource under a Map type which is illegal in SDKV2.
I don't understand why we'd want to use it here anyway -
tfs.Elem().(shim.Resource)is the correct way to check for an object typeThere 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.
I'm not going to block on this change but the Elem() representation is continuously confusing. I don't think "object type" is defined enough for me.
How about #2187 to try to clarify some of the issues here?
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.
My understanding was that
tfs.Elem().(shim.Resource)was insufficient to identify an object type:tfs.Type() == TypeList && tfs.Elem().(shim.Resource)implied aList[Object]wheretfs.Type() == TypeMap && tfs.Elem().(shim.Resource)implied anObjecttype.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.
When recurring over a List[Object] type which is indeed
tfs.Type() == TypeList && tfs.Elem().(shim.Resource)the code will call elemSchemas to compute a shim.Schema for the Object, which will result in a shim.Schema value that's not recognized by CastToTypeObject. One can argue that it should, in fact, be recognized, and then we could bring that back here and work this out that way. That's a bit more changes though.