Skip to content

FIX Close cleanly distributed Client at the end of unit tests#1526

Merged
ogrisel merged 2 commits intojoblib:masterfrom
fcharras:mnt/stabilize_dask_tests
Dec 1, 2023
Merged

FIX Close cleanly distributed Client at the end of unit tests#1526
ogrisel merged 2 commits intojoblib:masterfrom
fcharras:mnt/stabilize_dask_tests

Conversation

@fcharras
Copy link
Copy Markdown
Contributor

@fcharras fcharras commented Nov 23, 2023

The distributed pipeline seemed to be less stable after #1520 , all tests do pass and there is no deadlock but the python interpreter terminates with an error code and outputs a long traceback related to backend termination.

This PR might help fix that, it ensures that when a distributed.Client is opened locally in unit tests, it is also closed cleanly before exiting the test (like it has always been done in https://github.com/joblib/joblib/blob/master/joblib/test/test_dask.py but was overlooked in https://github.com/joblib/joblib/blob/master/joblib/test/test_parallel.py )

@fcharras fcharras force-pushed the mnt/stabilize_dask_tests branch from 1a5194e to dfd29c4 Compare November 23, 2023 13:26
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ebfe05d) 95.06% compared to head (3ea275c) 95.06%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1526      +/-   ##
==========================================
- Coverage   95.06%   95.06%   -0.01%     
==========================================
  Files          44       44              
  Lines        7564     7562       -2     
==========================================
- Hits         7191     7189       -2     
  Misses        373      373              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Contributor

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

Thanks very much. The test logs look much cleaner now!

@ogrisel ogrisel merged commit 6310841 into joblib:master Dec 1, 2023
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