Skip to content
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

Rescue exception in DMI plugin #952

Merged
merged 1 commit into from
Apr 5, 2017
Merged

Rescue exception in DMI plugin #952

merged 1 commit into from
Apr 5, 2017

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Feb 3, 2017

This gets us non-exploding output on Linux systems now. This was the
last plugin.

Signed-off-by: Tim Smith tsmith@chef.io

@tas50 tas50 requested review from btm and thommay February 3, 2017 10:40
@btm
Copy link
Contributor

btm commented Feb 3, 2017

Is it likely for dmidecode to fail for a reason other than it doesn't exist? if so, should we be including the exception in the debug output?

@tas50
Copy link
Contributor Author

tas50 commented Feb 10, 2017

I'm fairly certain this is the only failure. At least I haven't seen / heard of another failure with the dmi plugin

This gets us non-exploding output on Linux systems now. This was the
last plugin.

Signed-off-by: Tim Smith <tsmith@chef.io>
@thommay
Copy link
Contributor

thommay commented Apr 5, 2017

https://github.com/chef/ohai/pull/952/files?w=1 makes this review much clearer.

Copy link
Contributor

@smurawski smurawski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thommay that diff view magic is awesome.

Great cleanups @tas50

@tas50 tas50 merged commit fc6dcef into master Apr 5, 2017
@tas50
Copy link
Contributor Author

tas50 commented Apr 5, 2017

Great diff link

@tas50 tas50 added the Type: Bug Does not work as expected. label Apr 11, 2017
@tas50 tas50 deleted the dmi_explosion branch May 3, 2017 19:11
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants