-
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
Fix nested computed read #2181
Changes from 15 commits
0c07814
841998f
a2b4bf5
baa894f
efe595d
96a2f0b
48fd41b
41499a0
637504a
7e5b053
ff29def
bf92dfd
ec38a62
4399511
f0a277b
0260a54
886d4f2
4d31021
f9ce200
76191b9
32f9e37
39251cf
b5ce289
e40f6a0
4a59ef9
ec3ae31
1b672b5
7fac4a1
f07cf0c
8f9d753
4427b72
849961e
08fc395
89906ef
13521d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ func IsOfTypeMap(tfs shim.Schema) bool { | |
|
|
||
| // CastToTypeObject performs a checked cast from shim.Schema to a TF object (a collection | ||
| // of fields). | ||
| // Note that this is only valid for PF resources - in the TF SDK objects are not represented as maps | ||
|
||
| // | ||
| // See [shim.Schema.Elem()] comment for all the details of the encoding. | ||
| func CastToTypeObject(tfs shim.Schema) (shim.SchemaMap, bool) { | ||
|
|
||
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.