-
Notifications
You must be signed in to change notification settings - Fork 93
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
internal/fwserver: Use response plan value instead of request with nested attributes and blocks #669
Conversation
…sted attributes Reference: #644 Reference: https://discuss.hashicorp.com/t/framework-v1-1-1-strategies-for-diagnosing-resource-state-churn/49688 The attribute plan modification logic must preserve any changes made by the "top level" list/map/set/single nested attribute plan modifiers before continuing on to nested attributes. New unit testing failures prior to the logic change: ``` --- FAIL: TestAttributeModifyPlan (0.00s) --- FAIL: TestAttributeModifyPlan/attribute-list-nested-usestateforunknown (0.00s) attribute_plan_modification_test.go:1107: Unexpected response (-wanted, +got): fwserver.ModifyAttributePlanResponse{ - AttributePlan: s`[{"nested_computed":"statevalue1"}]`, + AttributePlan: s"[]", Diagnostics: nil, RequiresReplace: s"[]", Private: nil, } --- FAIL: TestAttributeModifyPlan/attribute-map-nested-usestateforunknown (0.00s) attribute_plan_modification_test.go:1107: Unexpected response (-wanted, +got): fwserver.ModifyAttributePlanResponse{ - AttributePlan: s`{"key1":{"nested_computed":"statevalue1"}}`, + AttributePlan: s"{}", Diagnostics: nil, RequiresReplace: s"[]", Private: nil, } --- FAIL: TestAttributeModifyPlan/attribute-single-nested-usestateforunknown (0.00s) attribute_plan_modification_test.go:1107: Unexpected response (-wanted, +got): fwserver.ModifyAttributePlanResponse{ - AttributePlan: s`{"nested_computed":"statevalue1"}`, + AttributePlan: s`{"nested_computed":<unknown>}`, Diagnostics: nil, RequiresReplace: s"[]", Private: nil, } --- FAIL: TestAttributeModifyPlan/attribute-set-nested-usestateforunknown (0.00s) attribute_plan_modification_test.go:1107: Unexpected response (-wanted, +got): fwserver.ModifyAttributePlanResponse{ - AttributePlan: s`[{"nested_computed":"statevalue1"}]`, + AttributePlan: s"[]", Diagnostics: nil, RequiresReplace: s"[]", Private: nil, } FAIL ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Are equivalent changes required for:
Looking at their code, that does seem to be case. Will make the equivalent changes there. |
I've just rebuilt my provider with this version of the framework, can confirm that this fix addressed the issue I was having. Thanks very much, @bflad! |
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 #644
Reference: https://discuss.hashicorp.com/t/framework-v1-1-1-strategies-for-diagnosing-resource-state-churn/49688
The attribute and block plan modification logic must preserve any changes made by the "top level" list/map/set/single nested attribute/block plan modifiers before continuing on to nested attributes/blocks.
New unit testing failures prior to the logic change: