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>
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