refactor terraform scripts and apply/destroy cli commands #126
Conversation
| if not return_output: | ||
| subprocess.run(["terraform", *cmd], check=False) | ||
| proc = subprocess.Popen( | ||
| ["terraform", *cmd], | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| ) | ||
| stdout, stderr = proc.communicate() | ||
| return proc.returncode, stdout.decode("utf-8"), stderr.decode("utf-8") |
There was a problem hiding this comment.
should we pipe out the outputs and stderr from terraform? we want the users to know exactly what terraform is doing write?
There was a problem hiding this comment.
Doesn't user already see the changes in their console with subprocess?
There was a problem hiding this comment.
ohh the issue was there were two subprocess calls happening if return_output if false.
| terraform_run(["apply", "-destroy", "-var-file", TERRAFORM_VALUES_FILE, '-auto-approve']) | ||
|
|
There was a problem hiding this comment.
having -auto-approve here would not give the users a change to see exactly what changes are being made to the infra right? They should probably have a plan phase before destroy
There was a problem hiding this comment.
Yes. It won't let user to see what changed. I am not sure user can interact with subprocess. Also I think auto approve could be an Orion in our cli.
There was a problem hiding this comment.
since we don't pipe the stdin the user can interact with it as if we called it from the terminal, so it will be a low friction way to perform all the terraform commands and yeah, we should have the auto-approve command or a more general command like '--yes' or something.
Description
Motivation and Context
How Has This Been Tested?
Checklist:
make formatandmake lintscript have passed