Skip to content
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

Add dynamic type support documentation and considerations (feature PR) #954

Merged
merged 18 commits into from
Mar 20, 2024

Conversation

austinvalle
Copy link
Member

This PR is targeting #931 for ease of review/comments

@austinvalle austinvalle requested a review from a team as a code owner March 14, 2024 23:45
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure if we should include some example scenarios on when you should use a dynamic type? Does anyone have a list of those exact scenarios that couldn't be covered by one or more static types?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some Discuss threads in which the suggested solution has been, to this point, to use terraform-plugin-go with muxing in a provider to obtain dynamic type support:

However, I'm not sure if these are cases that we should highlight as being scenarios in which we encourage the usage of dynamic types and values. Interested to hear other opinions on this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open question: Should I update the benefits page in terraform-docs-common for this feature? Do we want to advertise it? 😆

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favour of adding this feature as a benefit, perhaps with a caveat relating to the extra care that needs to be taken when using dynamic types. Maybe a link through to a page that contains a more complete list of the cases where using dynamic types are warranted and the gotchas that need to be considered when doing so?

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Great documentation, provides a great level of detail and clarity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favour of adding this feature as a benefit, perhaps with a caveat relating to the extra care that needs to be taken when using dynamic types. Maybe a link through to a page that contains a more complete list of the cases where using dynamic types are warranted and the gotchas that need to be considered when doing so?

In this example, a known dynamic value is created, where the underlying value is a known object value:

```go
elementTypes := map[string]attr.Type{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth either adding some verbiage here around why a provider developer might want to do this, or linking to another page that provides information around when it makes sense to utilise dynamic types and values?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some Discuss threads in which the suggested solution has been, to this point, to use terraform-plugin-go with muxing in a provider to obtain dynamic type support:

However, I'm not sure if these are cases that we should highlight as being scenarios in which we encourage the usage of dynamic types and values. Interested to hear other opinions on this.


Practitioner configuration editor integrations, like the Terraform VSCode extension and language server, cannot provide any static information when using dynamic data in configurations. This can result in practitioners using dynamic data in expressions (like [`for`](/terraform/language/expressions/for)) incorrectly that will only error at runtime.

Given this example, a resource schema defines a top level computed [dynamic attribute](/terraform/plugin/framework/handling-data/attributes/dynamic) named `example_attribute`:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a separate heading here warranted? Although this is using the example of the usage of for that appears under "Downstream Tooling", perhaps this should appear in a sub-section of examples?

}
```

When writing test configurations and debugging provider issues, developers will also want to understand how Terraform represents [complex type literals](/terraform/language/expressions/type-constraints#complex-type-literals). For example, Terraform does not provide any way to directly represent lists, maps, or sets.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked page provides an excellent level of detail, I'm just wondering whether it might be worth saying something about the relationship between framework types, and the Terraform representation of complex type literals, as the former will likely be familiar to provider developers, but the latter may not be.

}
```

Then the exact type must be planned and stored in state during `apply` as a [list](/terraform/plugin/framework/handling-data/types/list) of strings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps expand this section to say a little more, or provide examples, about the handling/behaviour of Terraform list/tuple, map/object, and framework list, map, object and set types in this regard?

Copy link
Member

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial feedback -- please reach out with questions/comments/concerns.

website/docs/plugin/framework/handling-data/paths.mdx Outdated Show resolved Hide resolved
@bflad bflad added the documentation Improvements or additions to documentation label Mar 19, 2024
@bflad bflad added this to the v1.7.0 milestone Mar 19, 2024
Copy link
Member

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me! 🚀 I'm personally okay if we create followup documentation issues for additional information, since this captures the overall essence of the feature.

@austinvalle
Copy link
Member Author

We're going to merge this as an MVP for the docs. Potential benefits and use-cases/recommendations can be added in a follow-up PR

@austinvalle austinvalle merged commit 4173c54 into av/dynamic-support Mar 20, 2024
2 checks passed
@austinvalle austinvalle deleted the av/dynamic-docs branch March 20, 2024 21:33
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants