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

[system][wmi_check] handle TimeoutException #2228

Merged
merged 2 commits into from Feb 12, 2016
Merged

Conversation

yannmh
Copy link
Member

@yannmh yannmh commented Jan 27, 2016

[core][wmi] list to cache connections

Use a list to cache WMI connections instead of a set.

[system][wmi_check] handle TimeoutException

WMI queries can potentially raise a TimeoutException on query timeouts. Handle it.

Use a `list` to cache WMI connections instead of a `set`.
WMI queries can potentially raise a `TimeoutException` on query
timeouts. Handle it.
@yannmh yannmh added this to the 5.7.0 milestone Jan 27, 2016
@truthbk
Copy link
Member

truthbk commented Jan 27, 2016

Looks good, just as we had discussed... my only question here is where do we want to call pythoncom.CoInitialize() - one option could be _query(), the other would be in get_connection() - note that I believe that when you call CoInitialize() you must also call Dispatch() and ConnectServer() to get a new live connection. IMHO this should be in get_connection() and let the old connection get garbage collected... otherwise the sampler becomes kind of cumbersome to use.

The thing then would be, it wouldn't make too much sense to cache the connection if in reality we actually create a new one every time we call get_connection().... 😕

Almost certainly connection here will be closed and we'd need to call CoInitialize(), Dispatch(), etc... again

yannmh added a commit that referenced this pull request Feb 12, 2016
[system][wmi_check] handle `TimeoutException`
@yannmh yannmh merged commit 6286faa into master Feb 12, 2016
@yannmh yannmh deleted the yann/wmi-fix-timeout branch February 12, 2016 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants