-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Interpolate variable during Input and Validate #6833
Conversation
Variables weren't being interpolated during the Input phase, causing a syntax error on the interpolation string. Adding `walkInput` to the EvalTree operations prevents skipping the interpolation step.
Adding walkValidate to the EvalTree operations, and removing the walkValidate guard from the Interpolater.valueModuleVar allows the values to be interpolated for Validate.
LGTM @jbardin! 🎉 |
Type: ast.TypeString, | ||
} | ||
return nil | ||
} |
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.
What I'd do here is walk back to the PR / commit where this stanza was introduced and determine what the intent was just to double check that we're not reversing some prior decision.
The test suite should cover us, so the fact that everything is passing is probably enough, but I think for such a subtle area of the code base it's worth it to do extra due diligence.
Let me know what the git archeology turns up!
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 wanted to make sure this case was handled, and it is! So here's an extra test for us.
We hashed this out in Slack, I submitted one extra test case I was concerned about. If you pull that into your branch this LGTM for merge! 🚢 |
c961446
to
b205ac2
Compare
@jbardin @phinze , should this be fixed in 0.7.0rc2? I think I am running into this issue, and if I replace the parameter with the actual contents of the module output, it works fine. I am using the output of one module as input for another module, and then I get this error on the count:
|
@ddegoede, yes that patch should be in rc2. I wonder if this is somehow a different manifestation of the same problem. Can you provide a standalone example that reproduces this in a new issue? Thanks! |
@jbardin I created an new issue for my error with TF plan and modules |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
fixes #5322