tfsdk: Update Config, Plan, and State type GetAttribute methods to use reflection #167
Conversation
bflad
added
enhancement
| for idx, valueAsDiag := range valueAsDiags { | ||
| if valueAsDiag.Severity() == diag.SeverityError { | ||
| valueAsDiags[idx] = diag.NewAttributeErrorDiagnostic( | ||
| path, | ||
| valueAsDiag.Summary(), | ||
| valueAsDiag.Detail(), | ||
| ) | ||
| } else if valueAsDiag.Severity() == diag.SeverityWarning { | ||
| valueAsDiags[idx] = diag.NewAttributeWarningDiagnostic( | ||
| path, | ||
| valueAsDiag.Summary(), | ||
| valueAsDiag.Detail(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Eek. Can we add an issue at least to have diag.Diagnostic have a SetPath method to avoid this kind of stuff?
There was a problem hiding this comment.
I created #169, but it might require some refactoring to the diag package.
tfsdk/config.go
Outdated
| func (c Config) GetAttribute(ctx context.Context, path *tftypes.AttributePath, target interface{}) diag.Diagnostics { | ||
| attrValue, diags := c.getAttributeValue(ctx, path) | ||
|
|
||
| if attrValue == nil || diags.HasError() { |
There was a problem hiding this comment.
I'm still 🤔 about the case where attrValue is nil. If error diagnostics are returned, the provider will think that target has been set, when it has not. If error diagnostics are returned, then the check is unnecessary. The only case in which the check helps us avoids a panic but is really hard to debug.
Do we want to catch a nil attrValue and return an error diagnostic for it, instead?
paddycarver
added
the
reflection
|
This may require some light rebase work depending on #185 |
…e reflection Reference: #134
…O for diagnostic SetPath
bflad
force-pushed
the
bflad-tfsdk-GetAttribute-reflection
branch
from
f8cc3a2 to
862229f
Compare
|
Rebased after #185 |
|
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. |
Closes #134