-
Notifications
You must be signed in to change notification settings - Fork 219
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
crash_reports: add optional timeout to get_new_sysdiagnose
#776
crash_reports: add optional timeout to get_new_sysdiagnose
#776
Conversation
d0fae21
to
a5789fb
Compare
a5789fb
to
0ee8664
Compare
0ee8664
to
06de161
Compare
* on timeout raise error `SysdiagnoseTimeoutError`
06de161
to
a7ed5be
Compare
|
||
@pytest.mark.parametrize( | ||
('end_time', 'return_value'), | ||
((-1, True), (0, True), (10, False), (None, False)) |
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.
this test fails
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.
Hey @doronz88 , is there a way that I can contribute to this PR?
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.
Feel free to fix this test yourself
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.
Ok, but that will create a new PR, won't it? I was thinking on this PR, which I cannot edit (or it seems so)
Hey @StephenGemin , I find this PR very interesting to be added. But I just realised that the PR #847 I submitted, even if complementary, may yield conflicts when trying to incorporate yours. Please take a look to the new code of get_new_sysdiagnose. Looking forward to having your PR on the master branch! |
If there are any issues collecting sysdiagnose archive, it could potentially hang at this API. This provides a backwards compatible way for fully automated (and unmonitored) test applications to continue with execution.
Updates
timeout
keyword toget_new_sysdiagnose
signatureSysdiagnoseTimeoutError