spec: Add ObjectType with CustomType #26
Merged
Conversation
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
Reference: #25 This breaking change ensures there is consistency that specification implementations can use `CustomType` with all types, in this case object types underlying any attribute type. For existing implementations, such as: ```json { "name": "list_object_attribute", "list": { "computed_optional_required": "computed", "element_type": { "object": [ { "name": "obj_string_attr", "string": {} } ] } } }, ``` Are now defined as: ```json { "name": "list_object_attribute", "list": { "computed_optional_required": "computed", "element_type": { "object": { "attribute_types": [ { "name": "obj_string_attr", "string": {} } ] } } } }, ``` Note that there is now a slight inconsistency between how `CustomType` is defined on collection types (`ListType`, `MapType`, and `SetType`) that are already element types and need to include their own element type. The `CustomType` is specified along side the element type, rather than explicitly having an `ElementType` delineation. Currently: ```json { "name": "map_list_string_attribute", "map": { "computed_optional_required": "computed", "element_type": { "list": { "custom_type": { "import": "github.com/hashicorp/terraform-plugin-framework/types/basetypes", "type": "basetypes.ListType", "value_type": "basetypes.ListValue" }, "string": {} } } } }, ``` It may make sense to reintroduce what was originally designed in the specification with the verbose `ElementType` property for clarity. ```json { "name": "map_list_string_attribute", "map": { "computed_optional_required": "computed", "element_type": { "list": { "custom_type": { "import": "github.com/hashicorp/terraform-plugin-framework/types/basetypes", "type": "basetypes.ListType", "value_type": "basetypes.ListValue" }, "element_type": { "string": {} } } } } }, ```
This was referenced Jun 21, 2023
austinvalle
approved these changes
Jun 21, 2023
Member
austinvalle
left a comment
austinvalle
left a comment
There was a problem hiding this comment.
Makes sense to me 🚀 - I opened hashicorp/terraform-plugin-codegen-openapi#17 to update the OpenAPI generator
bflad
added a commit
that referenced
this pull request
Jun 22, 2023
…ect property Reference: #26 For consistency with collection attribute types which use an explicit `element_type` object property and object types which use an explicit `attribute_types` property. An additional benefit is that it further distinguishes where a `custom_type` may apply. Previous implementations: ```json { "name": "list_map_attribute", "list": { "computed_optional_required": "computed", "element_type": { "map": { "string": {} } } } }, ``` Are now defined as: ```json { "name": "list_map_attribute", "list": { "computed_optional_required": "computed", "element_type": { "map": { "element_type": { "string": {} } } } } }, ``` The Go bindings and any consuming code do not require changes since Go implicitly required the `ElementType` field definition, even when it was using type embedding.
bflad
added a commit
that referenced
this pull request
Jun 22, 2023
…ect property (#29) Reference: #26 For consistency with collection attribute types which use an explicit `element_type` object property and object types which use an explicit `attribute_types` property. An additional benefit is that it further distinguishes where a `custom_type` may apply. Previous implementations: ```json { "name": "list_map_attribute", "list": { "computed_optional_required": "computed", "element_type": { "map": { "string": {} } } } }, ``` Are now defined as: ```json { "name": "list_map_attribute", "list": { "computed_optional_required": "computed", "element_type": { "map": { "element_type": { "string": {} } } } } }, ``` The Go bindings and any consuming code do not require changes since Go implicitly required the `ElementType` field definition, even when it was using type embedding.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Closes #25
This breaking change ensures there is consistency that specification implementations can use
CustomTypewith all types, in this case object types underlying any attribute type.For existing implementations, such as:
{ "name": "list_object_attribute", "list": { "computed_optional_required": "computed", "element_type": { "object": [ { "name": "obj_string_attr", "string": {} } ] } } },Are now defined as:
{ "name": "list_object_attribute", "list": { "computed_optional_required": "computed", "element_type": { "object": { "attribute_types": [ { "name": "obj_string_attr", "string": {} } ] } } } },Note that there is now a slight inconsistency between how
CustomTypeis defined on collection types (ListType,MapType, andSetType) that are already element types and need to include their own element type. TheCustomTypeis specified along side the element type, rather than explicitly having anElementTypedelineation.Currently:
{ "name": "map_list_string_attribute", "map": { "computed_optional_required": "computed", "element_type": { "list": { "custom_type": { "import": "github.com/hashicorp/terraform-plugin-framework/types/basetypes", "type": "basetypes.ListType", "value_type": "basetypes.ListValue" }, "string": {} } } } },It may make sense to reintroduce what was originally designed in the specification with the verbose
ElementTypeproperty for clarity.{ "name": "map_list_string_attribute", "map": { "computed_optional_required": "computed", "element_type": { "list": { "custom_type": { "import": "github.com/hashicorp/terraform-plugin-framework/types/basetypes", "type": "basetypes.ListType", "value_type": "basetypes.ListValue" }, "element_type": { "string": {} } } } } },