tfsdk: Initial support for Attribute, Data Source, Provider, and Resource validation #75
Conversation
| ) | ||
|
|
||
| // DataSourceConfigValidator describes reusable DataSource configuration validation functionality. | ||
| type DataSourceConfigValidator interface { |
There was a problem hiding this comment.
Hot take: is Config necessary here, or can we save some typing by removing it?
There was a problem hiding this comment.
I'm certainly amenable to updating the name, I'm just not sure if there's any concerns with keeping the names and types relatively close to the RPCs similar to as has been done elsewhere. Should there be any future other implementations with differing semantics, it could be weird to have DataSourceValidator vs DataSourceStateValidator for example.
|
I'm going to address the above comments and pull in the |
Co-authored-by: Paddy <paddy@hashicorp.com>
…d ResourceConfigValidator description comments
… ValidateResourceConfigRequest
|
This now includes the initial |
bflad
changed the title
|
|
||
| attribute.validate(ctx, attributeReq, attributeResp) | ||
|
|
||
| resp.Diagnostics = attributeResp.Diagnostics |
There was a problem hiding this comment.
Should this append instead, in case multiple attribute validators return warning diags?
There was a problem hiding this comment.
Initially this was appending, but I was asked to switch it to "passthrough" diagnostics: #75 (comment)
Personally I think pre-populating a response, while offering overwrite capabilities easily, could be confusing for implementors to understand the semantics. We could offer prior diagnostics in the request with diagnostic modifications in the response to clarify things on both sides of these calls (or not allow diagnostic modification). Please let me know if you would like me to raise an issue regarding this.
|
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
Reference: #65