Introduce attr.TypeWithValidate handling to internal/reflect and tfsdk #82
Conversation
If it's cleaner/better that they return Diagnostics, I think that's at least worth exploring as an option. |
|
Something that might also be worth considering with this type of validation is what might happen if a type wants to introduce a non-backwards compatible change. Warning diagnostics are a good approach to prepare practitioners for such a change, but if in a major provider release the change occurs, errors during state reading operations could leave practitioners in a tough spot without first applying/refreshing an updated value on the older version. |
|
Hmmmm, yeah, that's a solid point. Would we want to introduce migrations in that case? Do we want to validate on read? I think we have to, right, to ensure we have the appropriate value for the type? 🤔 I think providers could work around it with state migrations, but that may not be ideal. |
Reference: #17 The next step will be deciding how to handle the returned diagnostics since there can be multiple warnings and/or errors, however these callers return a single error type.
bflad
force-pushed
the
f-attr-Type-Validate
branch
from
4a98028 to
0d1c015
Compare
…and state handling Required making the following diagnostics aware: - `internal/reflect` - `tfsdk.Config` - `tfsdk.Plan` - `tfsdk.State` - `(types.List).ElementsAs()` - `(types.Map).ElementsAs()` - `(types.Object).As()`
bflad
force-pushed
the
f-attr-Type-Validate
branch
from
9455177 to
5c6d44c
Compare
|
I still need to circle through all the places that are introducing the |
…ing types with validation
|
This is ready, @kmoe! |
Co-authored-by: Paddy <paddy@hashicorp.com>
This panic can happen when values received from `(tfsdk.State).Get()` are not fully realized due to errors.
There was a problem hiding this comment.
I think this looks good. Great work, @bflad. I think once merged we're committing ourselves to tackling #24 before release, though, as people will need to know, at the very least, if Get and Set (etc.) are returning errors they should abort on or warnings they should roll through. I think that's okay, though.
|
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. |
Reference: #17
The next step will be deciding how to handle the returned diagnostics since there can be multiple warnings and/or errors, however these callers return a single error type.