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.

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

Prevent ToTerraformValue panic when the underlying attr.Value types doesn't have type information for its fields #354

Merged

Conversation

detro
Copy link
Contributor

@detro detro commented May 27, 2022

ToTerraformValue in all the non primitive types of attr.Value, relies on it containing ElemType or AttrsType to do the conversion to TF types.

But if by mistake one omits setting that, the framework was throwing a panic as it wasn't checking for those fields being not null. Now it does.

@detro detro requested a review from a team as a code owner May 27, 2022 16:11
@detro detro added bug Something isn't working types Issues and pull requests about our types abstraction and implementations. labels May 27, 2022
@detro detro added this to the v0.9.0 milestone May 27, 2022
Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

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

LGTM

@detro detro force-pushed the detro/bubble_up_incomplete_attrValue_when_calling_ToTerraformValue branch from cfa83da to 0b84b7e Compare May 30, 2022 08:46
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Overall 👍 Just some small things for consideration 🚀

types/list_test.go Outdated Show resolved Hide resolved
types/list.go Outdated Show resolved Hide resolved
Ivan De Marino added 3 commits May 31, 2022 16:47
…ubble up an error (instead of panic) if the given struct is missing the `ElemType` / `AttrsType` field.
@detro detro force-pushed the detro/bubble_up_incomplete_attrValue_when_calling_ToTerraformValue branch from 0b84b7e to 402a4c9 Compare May 31, 2022 16:03
@detro detro merged commit b9dde4f into main May 31, 2022
@detro detro deleted the detro/bubble_up_incomplete_attrValue_when_calling_ToTerraformValue branch May 31, 2022 16:06
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working types Issues and pull requests about our types abstraction and implementations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants