-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: add --use-parameter-defaults flag #21119
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
Conversation
31577a6 to
5645473
Compare
To make it easier to add new tests and parameters. Largely unchanged in functionality except: 1. All tests now perform parameter validation. 2. Add types to the parameters where before they were all defaulting to strings (except boolean, which seems to be an existing issue).
This works already in production, but not in the tests because the generated Terraform was missing quotes around the default value, which must have been breaking the parser because it was returning no parameters. Then we were removing all non-matching parameters from the request, which was all of them since the parser returned none.
This will accept parameter defaults without prompting (whether those defaults come from the template or --parameter-default). Similar to --yes, but for parameter prompts.
5645473 to
8f3f003
Compare
|
@deansheather so we have a few solutions to this floating around, my proposal is that we merge this especially since it also refactors the tests and has a related bugfix, then I will open a new PR to also add a That means if you want to auto-select the defaults, but still be prompted for missing values or for a template, you can use this flag, if you want fully non-interactive you can use |
|
Since there's been 3 PRs from 3 different people trying to fix this issue we should probably get someone from product to chime in on what behavior they want. |
|
We got some feedback that this approach works so I think we go ahead and merge, and I will follow up with |
Emyrk
left a comment
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.
👍
Idk if there is a good spot in the docs for this
|
Lemme check up on the docs when I work on the next flag! |
This is like
--yes, but for parameter prompts. Closes #20815. Most of the diff is refactoring the tests since they were all doing the same setup and changes to markdown/golden files for the--yesdescription change, the actual code changes are fairly small 😛