-
Notifications
You must be signed in to change notification settings - Fork 400
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
Support persisting empty fields in StructToData #3362
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3362 +/- ##
==========================================
+ Coverage 83.43% 83.44% +0.01%
==========================================
Files 176 176
Lines 16219 16221 +2
==========================================
+ Hits 13532 13536 +4
+ Misses 1865 1864 -1
+ Partials 822 821 -1
|
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.
thank you!
We should be careful of APIs which "default" to empty structs / do not differentiate between not-set and empty struct. |
@hectorcast-db can we merge it, or we need to work on it further? |
I was concerned that this might exacerbate some issues with suppress diff, as it is will add empty dictionaries into state which might need diff suppression. However, I realized that that would only be needed if the API returned non-nil/empty values, which would always end up in the state anyways. So I think that isn't a concern, so we can merge this. |
Changes
Some APIs legitimately return empty objects in fields, but StructToData does not persist these objects (e.g. #3351). This is caused by a filter in collectionToMaps that removes data structures that don't have any non-optional fields or optional fields set to a non-empty/non-nil value.
This PR removes this check. I need to do a bit more work to verify the impact of removing this on the rest of the provider.
Tests
make test
run locallydocs/
folderinternal/acceptance