-
Notifications
You must be signed in to change notification settings - Fork 90
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
Check stdout even when exit_status is 0, and handle JSON.parse exception #601
Conversation
Hello CodesWhisperer! Thanks for the pull request! Here is what will happen next:
Thank you for contributing! |
Code looks reasonable to me, just need to get it past the linter. Also please sign your commit to indicate acceptance of our contribution terms - https://github.com/inspec/train/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco |
ca0e6e2
to
142c22a
Compare
@clintoncwolfe |
The gem ffi segfaults under windows right now. We have a pending PR to pin the version back to a working version. Once that merges you can rebase to master and the windows tests should pass. |
Hi @CodesWhisperer - This looks good, thank you! I left one comment suggesting to make the code easier to understand; because in future an issue may occur with some other platform; and the logic might get changed in a way that fixes that other platform but causes Cisco to start failing again... As Clinton said, please rebase this branch against master to get the Windows tests working. Thanks |
@CodesWhisperer Can you rebase this against master please? |
It would be good to get this merged. Could you rebase against master @CodesWhisperer ? |
When the target is a cisco_ios device, the exit_status is 0 but the error is described in the stdout, so the if condition doesn't work and the JSON.parse raises an exception. The JSON.parse should always be in rescue block. Signed-off-by: CodesWhisperer <[email protected]>
Signed-off-by: CodesWhisperer <[email protected]>
142c22a
to
ce73706
Compare
@CodesWhisperer unfortunately your last commit is missing your DCO, could you please push another commit with your DCO added? |
Signed-off-by: CodesWhisperer <[email protected]>
@rmoles Is it good? |
Awesome thanks @CodesWhisperer. Unfortunately, I don't have merge rights on this repo. Perhaps, @james-stocks or @Schwad can merge this. |
Thanks for this @CodesWhisperer ! |
Description
When the target is a cisco_ios device, the exit_status is 0 but the error is described in the stdout, so the if condition doesn't work and the JSON.parse raises an exception.
Also, the JSON.parse should always be in rescue block.
Related Issue
#599
Types of changes
Checklist: