-
Notifications
You must be signed in to change notification settings - Fork 154
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
fix: invocation of getEngineVersion returns an error #4047
Conversation
A patch was introduced via #3757 due to [a bug in pulumi-terraform-bridge](pulumi/pulumi-terraform-bridge#1809). The underlying bridge issue has now been fixed so I am removing the patch and restoring the original terraform behavior. fixes #4043
Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
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.
I'm a little unclear on the context here:
We introduced a patch to fix a panic; did that patch always produce this error? or did it start producing the error after we picked up the tfbridge change that fixed the underlying cause for the panic?
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.
The change lgtm.
I think for the future we need to be more careful with adding more tests for patches and following up with removing them when the underlying issue is fixed.
In this case the bridge fix was released 3 weeks ago, but we're missing some sort of automation to make us aware of this
I am not 100% sure. There were a couple of bug fixes in upstream that we had to revert/incorporate into our revert/patch. We had a test for the panic that was still passing, but it wasn't testing this specific use case with other properties provided. Since the linked issue says since
Yeah it would be nice to have automation to gather linked issues and label them with |
/release patch |
A patch was introduced via #3757 due to a bug in
pulumi-terraform-bridge.
The underlying bridge issue has now been fixed so I am removing the patch and restoring the original terraform behavior.
fixes #4043