-
Notifications
You must be signed in to change notification settings - Fork 334
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
[cli][bug] catch uninstalled terraform errors properly #202
Conversation
@@ -59,6 +59,9 @@ def run_command(runner_args, **kwargs): | |||
except subprocess.CalledProcessError as err: |
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.
can we just catch a tuple of error types and use the same CLI message?
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.
@jacknagz OSError
does not have a cmd
property - so we could, but we would lose some granularity. Doing it this way has the added bonus of knowing what command caused the OSError
(see: runner_args[0]
)
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.
1 quick inline comment
96014b5
to
b6ffa9e
Compare
to @jacknagz
cc @airbnb/streamalert-maintainers , @zbuc
resolves: #200
size: small
fix
OSError
when trying to run a command that does not exist. Previously,subprocess.CalledProcessError
would not catch this because commands must exit cleanly with an error code for that exception to be caught. Commands that do not exist do not exit with an error code, and thus this was missed.terraform_check
fails to validate a TF install, we will now exit execution of the cli.other changes