Skip to content

TST add test for recursive_terminate with nested executor#148

Merged
ogrisel merged 13 commits intomasterfrom
TST_kill_recursive
Aug 23, 2018
Merged

TST add test for recursive_terminate with nested executor#148
ogrisel merged 13 commits intomasterfrom
TST_kill_recursive

Conversation

@tomMoral
Copy link
Copy Markdown
Contributor

This fix the issue reported in joblib/joblib#676

@tomMoral tomMoral force-pushed the TST_kill_recursive branch 2 times, most recently from 389b04a to ea8ee85 Compare August 20, 2018 16:51
@tomMoral
Copy link
Copy Markdown
Contributor Author

psutil seems to fail to introspect children in travis....
Not sure if we want to use it by default, as introduced in #142, or revert to using pgrep and taskill by default and fall back to psutil when it is not working/not installed.

The main issue with default psutil is that we don't know when it is not working. Also, this problem seems to only arise with python3.4/3.5/3.6, I am not sure why it does not happen in 2.7/3.7.

initializer=_executor_mixin.initializer_event,
initargs=(_executor_mixin._test_event,))
assert executor.submit(sleep_and_return, 0, 42).result() == 42
if depth == 0:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name makes this test hard to understand: I would have expected depth to increase in the inner/nested calls.

f = self.executor.submit(self._test_recursive_kill)
_executor_mixin._test_event.wait()
# Give some time to make sure psutil will detect the child workers
time.sleep(.5)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer required, right?

@tomMoral tomMoral force-pushed the TST_kill_recursive branch from af89401 to 2de9b26 Compare August 22, 2018 13:36
@ogrisel ogrisel merged commit 3f31ee9 into master Aug 23, 2018
@ogrisel ogrisel deleted the TST_kill_recursive branch August 23, 2018 07:44
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.

2 participants