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

Catch signal handler change error #1147

Merged
merged 2 commits into from
Jan 18, 2023

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Jan 17, 2023

The interruptible signal handle was incompatible with dask, it would trigger the following error:

Exception: "ValueError('signal only works in main thread of the main interpreter')"

This PR fixes the problem by catching the error and keeping the original signal handler in that case.

@tfeher tfeher requested a review from a team as a code owner January 17, 2023 11:03
@tfeher tfeher self-assigned this Jan 17, 2023
@tfeher tfeher added bug Something isn't working non-breaking Non-breaking change and removed python labels Jan 17, 2023
@tfeher
Copy link
Contributor Author

tfeher commented Jan 17, 2023

Credits to @achirkin who provided the patch, thanks!

@codecov-commenter
Copy link

Codecov Report

Base: 87.99% // Head: 87.99% // No change to project coverage 👍

Coverage data is based on head (0751657) compared to base (efd42c9).
Patch has no changes to coverable lines.

Additional details and impacted files
@@              Coverage Diff              @@
##           branch-23.02    #1147   +/-   ##
=============================================
  Coverage         87.99%   87.99%           
=============================================
  Files                21       21           
  Lines               483      483           
=============================================
  Hits                425      425           
  Misses               58       58           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tfeher tfeher changed the title Ignore error during signal handler change Catch signal handler change error Jan 17, 2023
@cjnolet
Copy link
Member

cjnolet commented Jan 18, 2023

/merge

@rapids-bot rapids-bot bot merged commit 187ff9e into rapidsai:branch-23.02 Jan 18, 2023
@tfeher tfeher deleted the bug-interruptible-dask branch May 1, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants