FIX raise iterator exception in user's thread#1491
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1491 +/- ##
==========================================
- Coverage 94.94% 94.87% -0.07%
==========================================
Files 45 45
Lines 7492 7515 +23
==========================================
+ Hits 7113 7130 +17
- Misses 379 385 +6
☔ View full report in Codecov by Sentry. |
| islice = list(itertools.islice(iterator, big_batch_size)) | ||
| try: | ||
| islice = list(itertools.islice(iterator, big_batch_size)) | ||
| except Exception as e: |
There was a problem hiding this comment.
Should we try to catch KeyboardInterrupt ?
Maybe catch BaseException, but not StopIteration ?
There was a problem hiding this comment.
Hum, not sure about how this would change the behavior here...
What happens on keyboard interrupt for now?
I am in favor of keeping this simple for this PR and we can improve this in a follow up PR if necessary?
There was a problem hiding this comment.
I am in favor of keeping this simple for this PR and we can improve this in a follow up PR if necessary?
Yes let's do that.
I thought KeyboardInterrupt will be caught by whatever thread is being executed at that time but I don't find clear explanation of that, it might only be the main thread. I don't recall hearing from issues with keyboardinterrupt being raised here anyway so my suggestion is probably off.
|
LGTM ? |
Co-authored-by: Franck Charras <29153872+fcharras@users.noreply.github.com>
ogrisel
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the fix!
This PR ensures the errors raised by the iterator generating the task in
joblibare raised in the user space thread and not in internal backend threads.Fixes #1489