Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix conversion of unknown or null collections to empty in nested objects #161

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Fix conversion of unknown or null collections to empty in nested objects #161

merged 2 commits into from
Sep 24, 2024

Conversation

paaanic
Copy link
Contributor

@paaanic paaanic commented Sep 20, 2024

This PR fixes the problem mentioned in #160.

Thanks to @austinvalle for the code snippet that was left in the issue as an example and used as a basis for this fix.

@paaanic paaanic requested a review from a team as a code owner September 20, 2024 17:13
@hashicorp-cla-app HashiCorp CLA App
Copy link

hashicorp-cla-app bot commented Sep 20, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

austinvalle
austinvalle reviewed Sep 20, 2024
View reviewed changes
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @paaanic, everything here make sense, although we don't have a ton of robust acceptance tests outside of the snapshot-like tests we have. I think generally this should cover the original bug report, although I wouldn't be surprised if other custom type implementations have similar null/unknown handling issues.

Would you mind also creating a bugfix changelog for this PR? (this repo doesn't have a contributing document I'm seeing, but you can reference the plugin-framework one)

@austinvalle austinvalle added this to the v0.4.1 milestone Sep 20, 2024
@austinvalle austinvalle added the bug Something isn't working label Sep 20, 2024
@paaanic
Copy link
Contributor Author

paaanic commented Sep 21, 2024

Thank you for the quick review @austinvalle. Added a changelog entry.

@paaanic
Copy link
Contributor Author

paaanic commented Sep 24, 2024

It is worth noting that the same problem is mentioned in the #154, so this PR should fix it as well.

This was linked to issues Sep 24, 2024
Error: Provider produced invalid plan - planned value cty.ListValEmpty(cty.String) does not match config value cty.UnknownVal(cty.List(cty.String)) #160
Closed
Nested Maps - plan failure if the attribute is marked optional #154
Closed
austinvalle
austinvalle approved these changes Sep 24, 2024
View reviewed changes
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix!

I'll release this in v0.4.1 🚀

@austinvalle austinvalle merged commit c5a3592 into hashicorp:main Sep 24, 2024
5 checks passed
@austinvalle austinvalle mentioned this pull request Sep 24, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinvalle austinvalle austinvalle approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
v0.4.1
2 participants
@paaanic @austinvalle