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

tfgen.convert.Convert panics instead of returning an error #56

Closed
jkodroff opened this issue Mar 10, 2022 · 3 comments
Closed

tfgen.convert.Convert panics instead of returning an error #56

jkodroff opened this issue Mar 10, 2022 · 3 comments
Labels
impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec resolution/wont-fix This issue won't be fixed

Comments

@jkodroff
Copy link
Member

The code does the meat of converting HCL panics when it encounters an error, which complicates the code for downstream callers, like tfbridge.convertHCL():

https://github.com/pulumi/pulumi-terraform-bridge/blob/master/pkg/tf2pulumi/convert/tf12.go#L161-L167

If we panic for good reason, we need to document the reasoning in a comment. Otherwise, we should return an error in order to allow downstream callers to handle issues parsing HCL as they see fit.

@stack72
Copy link

stack72 commented Mar 21, 2022

@jkodroff I think you have changed this behaviour now right?

@jkodroff
Copy link
Member Author

@stack72 No - I have not touched any code in the tf2pulumi package. What I did was wrap the call in tfgen so that that particular call to convert.Convert() catches the panic.

@t0yv0
Copy link
Member

t0yv0 commented Feb 6, 2024

@guineveresaenger referenced this issue but I believe tf2pulumi codebase in the bridge is being deprecated in favor of calling pulumi for explicit conversion via pulumi-converter-terraform. I'm not aware that the converter can panic - it probably shouldn't or we just fix it if we find panics. So possibly we can close this as won't fix.

@t0yv0 t0yv0 added the resolution/wont-fix This issue won't be fixed label Mar 1, 2024
@t0yv0 t0yv0 closed this as completed Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec resolution/wont-fix This issue won't be fixed
Projects
None yet
Development

No branches or pull requests

5 participants